-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat(preprod): Add second row with build number, version info and date to comparison screen (EME-520) #104117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(preprod): Add second row with build number, version info and date to comparison screen (EME-520) #104117
Conversation
Add two-line layout for build comparison UI with enhanced metadata: - Display version, build number, and date on second line - Increase text size from sm to md for better readability - Add proper padding (lg) for improved visual spacing - Align metadata text with build information above
| ); | ||
| } | ||
|
|
||
| const StyledLinkButton = styled(LinkButton)` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to create this otherwise the LinkButton's height would be fixed and the elements would overflow.
| ); | ||
|
|
||
| // Build metadata parts for the second line | ||
| const metadataParts = [formattedDate]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the date isn't optional, everything else is.
| const dateToShow = dateBuilt || dateAdded; | ||
| const formattedDate = getFormattedDate( | ||
| dateToShow, | ||
| getFormat({timeZone: true, year: true}), | ||
| { | ||
| local: true, | ||
| } | ||
| ); | ||
|
|
||
| // Build metadata parts for the second line | ||
| const metadataParts = [formattedDate]; |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we always have at least the date_added so this shouldn't happen.
| } | ||
| if (buildNumber) { | ||
| metadataParts.unshift(`Build ${buildNumber}`); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Metadata order reversed from documented intent
The metadataParts array construction produces "Build number • Version • Date" but the PR description specifies "Version • Build number • Date". Using unshift adds each item to the beginning of the array, so adding version first then buildNumber results in buildNumber appearing first. To match the documented order, either reverse the order of the conditionals, or use push instead of unshift with the items already in the desired order.
| { | ||
| local: true, | ||
| } | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Missing date displays current date instead of fallback
When both dateBuilt and dateAdded are undefined or null, dateToShow becomes undefined. Passing undefined to getFormattedDate (which wraps moment.js) causes it to return the current date/time instead of handling the missing data case. This results in displaying today's date in the metadata line, which would mislead users about when the build was actually created.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we always have at least the date_added so this shouldn't happen.
Only show the build ID (#xxx) when there's no build_number to avoid redundant information in the UI.
chromy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
Summary
Adds a second line of information showing version build number and date.
I also adjust the logic to only show the buildId if the build number is missing since it was confusing the difference between the buildId and buildNumber when presented visually.
Before:

After:

Also on comparison selection screen:

Also light mode:

Changes
Fixes EME-520