feat(changelog): add changelog widget version comparison#614
feat(changelog): add changelog widget version comparison#614vivekv1504 wants to merge 4 commits intowebex:nextfrom
Conversation
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
rarajes2
left a comment
There was a problem hiding this comment.
Let's move all the business logic to a separate file comparison-view.js and import the methods in this file and use it. We can keep all the UI related methods in this file.
| </li> | ||
| <li> | ||
| <a href="?compareStableA=1.28.0&compareStableB=1.28.2&comparePackage=@webex/widgets&compareVersionA=1.28.2-next.2&compareVersionB=1.28.2-next.3" | ||
| >Package-level comparison - @webex/widgets specific versions</a |
There was a problem hiding this comment.
nitpick: please align the angular brackets
|
|
||
| /* Comparison Results Table */ | ||
| #comparison-results .table-wrapper { | ||
| max-height: 500px; |
There was a problem hiding this comment.
Let's use rem where recommended
| Handlebars.registerHelper('forIn', function (object) { | ||
| let returnArray = []; | ||
| for (let prop in object) { | ||
| returnArray.push({key: prop, value: object[prop]}); | ||
| } | ||
| return returnArray; | ||
| Handlebars.registerHelper("forIn", function(object) { | ||
| let returnArray = []; | ||
| for(let prop in object){ | ||
| returnArray.push({key: prop, value: object[prop]}); | ||
| } | ||
| return returnArray; |
There was a problem hiding this comment.
Let's revert all these formatting changes, it will reduce the diff. Applicable everywhere
| if (searchParams.package) { | ||
| if (!packageNameInputDropdown.disabled) { |
| versionPaths[version] = path; | ||
| optionsHtml += `<option value="${version}">${version}</option>`; | ||
| const populatePackageNames = (changelog) => { | ||
| let specialPackages = ['@webex/widgets', '@webex/cc-widgets']; |
There was a problem hiding this comment.
What are special packages ? Why they need to be treated differently from other packages ?
There was a problem hiding this comment.
this is in single search comparison that is written by old as format in sdk comparison and that special pacakges are shows separately while populated the pacages names ,that pacakages are popular or everywhere presented in stable. versions
| console.log('effectiveVersionA:', effectiveVersionA, '(requested:', versionASpecific, ')'); | ||
| console.log('effectiveVersionB:', effectiveVersionB, '(requested:', versionBSpecific, ')'); |
There was a problem hiding this comment.
We can remove all console logs
| if (comparisonPackageSelect) comparisonPackageSelect.value = ''; | ||
| if (versionAPrereleaseSelect) versionAPrereleaseSelect.value = ''; | ||
| if (versionBPrereleaseSelect) versionBPrereleaseSelect.value = ''; | ||
| if (comparisonPackageRow) comparisonPackageRow.style.display = 'none'; | ||
| if (prereleaseRow) prereleaseRow.style.display = 'none'; |
There was a problem hiding this comment.
Do we need to have the conditions ? We can reset the values irrespective of the conditions, right ?
| if (searchForm) searchForm.classList.remove('hide'); | ||
| if (comparisonForm) comparisonForm.classList.add('hide'); | ||
| if (comparisonResults) comparisonResults.classList.add('hide'); | ||
| if (searchResults) searchResults.classList.remove('hide'); | ||
| if (helperSection) helperSection.classList.remove('hide'); | ||
| if (comparisonHelper) comparisonHelper.classList.add('hide'); |
There was a problem hiding this comment.
Same here, see if we actually need the if conditions ?
Check this logic everywhere in this file
|
|
||
| const stableA = versionASelect.value; | ||
| const stableB = versionBSelect.value; | ||
| const selectedPackage = comparisonPackageSelect ? comparisonPackageSelect.value : null; |
There was a problem hiding this comment.
const selectedPackage = comparisonPackageSelect?.value;We can keep the logic like this and utilize the undefined value
| if (versionAPrereleaseSelect) versionAPrereleaseSelect.addEventListener('change', updateCompareButtonState); | ||
| if (versionBPrereleaseSelect) versionBPrereleaseSelect.addEventListener('change', updateCompareButtonState); |
There was a problem hiding this comment.
Do we have any logic to remove the event listeners when we switch to another mode? We should not be adding multiple listeners as we toggle through the single and comparison views.
Applicable everywhere
COMPLETES #https://jira-eng-gpk2.cisco.com/jira/browse/SPARK-770576
This pull request addresses
ADD VERSION COMAPRISON MODE FUNCTIONALITY IN WIDGETS
by making the following changes
Version Comparison Mode
Compare all packages between two stable versions
Package-level comparison with pre-release version support
Visual status indicators (Unchanged, Changed, Added, Removed)
Statistics dashboard (total packages, changed count, etc.)
Copy comparison link for sharing
Change Type
The following scenarios were tested
< ENUMERATE TESTS PERFORMED, WHETHER MANUAL OR AUTOMATED >
The GAI Coding Policy And Copyright Annotation Best Practices
Checklist before merging
Make sure to have followed the contributing guidelines before submitting.