Conversation
2850cb0 to
cc502d9
Compare
84dfd5f to
d586054
Compare
3ef53b3 to
ecfb549
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the releases confirmation UI to surface the progressive release slider more prominently, consolidates related components, and removes obsolete code and styles.
- Consolidates all progressive release controls into a new
ProgressiveBarControlcomponent. - Replaces legacy
ProgressiveRowGroup/progressiveRow/globalRowwith simplerReleaseRowGroupand moves button toggles. - Removes unused SCSS selectors and updates button markup to “Review changes” / “Hide changes.”
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| static/sass/_snapcraft_release.scss | Removed unused .p-releases-confirm__details-toggle and .p-releases-confirm__actions styles. |
| static/sass/_snapcraft_p-release-details-row.scss | Switched release-details grid to flex layout and added .progressive-release-control classes. |
| static/js/publisher/pages/Releases/components/releasesTable/cellViews.js | Added optional chaining for progressive data in tooltip. |
| static/js/publisher/pages/Releases/components/releasesConfirmDetails/types.js | Capitalized type strings for display consistency. |
| static/js/publisher/pages/Releases/components/releasesConfirmDetails/releaseRowGroup.js | Added new ReleaseRowGroup to list simple release rows. |
| static/js/publisher/pages/Releases/components/releasesConfirmDetails/releaseRow.js | Simplified ReleaseRow by removing legacy progress/notes props. |
| static/js/publisher/pages/Releases/components/releasesConfirmDetails/index.js | Swapped out old progressive row groups for ReleaseRowGroup and integrated ProgressiveBarControl. |
| static/js/publisher/pages/Releases/components/releasesConfirmActions.js | Added u-align--right to action buttons container. |
| static/js/publisher/pages/Releases/components/releasesConfirm.tsx | Merged toggle and action buttons into one bar, updated labels and conditional rendering. |
| static/js/publisher/pages/Releases/components/progressiveBarControl.js | Introduced new ProgressiveBarControl component for global slider. |
Comments suppressed due to low confidence (3)
static/js/publisher/pages/Releases/components/releasesConfirmDetails/index.js:35
- Calling
updateProgressiveReleasePercentagedirectly will just return the action object without dispatching it. You need to dispatch this action (e.g., via props fromconnectoruseDispatch) to actually update the Redux store.
updateProgressiveReleasePercentage(percentage);
static/js/publisher/pages/Releases/components/releasesTable/cellViews.js:77
- If
revisionorrevision.progressiveis undefined,revision?.progressive?.percentageisundefinedandMath.round(100 - undefined)yieldsNaN. Consider adding a default fallback (e.g.,|| 0) to ensure a number is passed toMath.round.
{Math.round(100 - revision?.progressive?.percentage)}%
static/js/publisher/pages/Releases/components/progressiveBarControl.js:1
- The new
ProgressiveBarControlcomponent contains interactive logic but lacks unit or snapshot tests. Adding tests would help prevent regressions in the slider behavior.
import React from "react";
| display: grid; | ||
| grid-column-gap: 1rem; | ||
| grid-template-columns: 4rem 15rem 2rem auto 16rem; | ||
| display: flex; |
There was a problem hiding this comment.
Good point - I've changed it to a simple p
There was a problem hiding this comment.
My bad - I hadn't pushed 🙄
As these aren't related to the changes introduced in this PR I've made separate tickets for those: |
Ah I see, I'm getting that too. I've tested on production and that seems to be an existing bug so I've made a separate ticket for that here: https://warthogs.atlassian.net/browse/WD-22672 |
| }) => ( | ||
| <div className="p-release-details-row"> | ||
| const ReleaseRow = ({ type, revisionInfo, channel }) => ( | ||
| <p> |
bartaz
left a comment
There was a problem hiding this comment.
Little issue with missing space, looks good apart from that.
…llViews.js Co-authored-by: Eduard M <4310497+edisile@users.noreply.github.com>
97b79b3 to
d1aca1c
Compare











Done
How to QA
Testing
Issue / Card
Fixes https://warthogs.atlassian.net/browse/WD-21913
Screenshot