SL-1900: VersionHistory legacy buttons and text to MUI (scope reduced)#73107
Draft
ebeastlake wants to merge 10 commits into
Draft
SL-1900: VersionHistory legacy buttons and text to MUI (scope reduced)#73107ebeastlake wants to merge 10 commits into
ebeastlake wants to merge 10 commits into
Conversation
Scope reduced from the full ticket (per discussion with reviewer): this PR only swaps raw <button> and text tags to MUI Button + Typography. Wrapping the panel in a DSCO dialog, swapping rows to DSCO SimpleList, and refactoring the StudioApp.js modal plumbing are deferred to follow-up tickets (see PR description). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two fixes on the swap: * "Latest Version" and disabled "View" were rendering as faint MuiButton disabled-text. They are status labels, not buttons -- the legacy code wrapped them in <button disabled> only as a styling hack. Render as MuiTypography body1 with secondary text color so they are readable. * MuiButton has no built-in sibling margin (Bootstrap btn did). Wrap the row's actions in MuiStack with spacing. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The Typography labels were rendering as <p>, which hit a legacy
global rule (apps/style/common.scss:1170, `#showVersionsModal p {
margin: 0 }`) that wins on specificity over MUI Stack's
margin-based sibling spacing -- buttons still got spacing because
they were not <p>, but the labels collapsed flush.
* Render the status-label Typography as <span> so the legacy `p`
rule does not match.
* Switch MuiStack to useFlexGap so spacing uses CSS gap (immune
to global margin overrides) instead of injected sibling margin.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two fixes: * The non-selected View was MuiButton component="a", rendering as <a class="MuiButton-...">. The global a:hover rule in apps/style/common.scss:66-72 sets `background: none` and wins on stylesheet order over MUI's hover bg -- so on hover the button went transparent with white text on a white modal, i.e. invisible. Restore the legacy <a><MuiButton> wrapper so the a:hover rule applies to the outer link, not the inner button. * Per reviewer: render disabled View in the selected row as a disabled MuiButton (variant="outlined" color="secondary") for layout consistency with the active rows, instead of as MuiTypography. "Latest Version" stays as MuiTypography. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* Latest Version typography: drop color=text.secondary so it renders in default primary text color (per reviewer). * Disabled View: switch from outlined to contained (color secondary, disabled) for higher contrast against the row bg. * Add versionRow.module.scss with padding-right on the action cell so the selected-row highlight extends past the View button's right edge. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
align Start Over to View * Latest Version typography: variant body2 (1rem/16px) so it matches the MUI Button medium font size next to it. * Disabled View: override Mui-disabled colors in the module to --background-neutral-tertiary + --text-neutral-primary (was --background-neutral-disabled + --text-neutral-disabled-inverse, which failed a11y contrast). * Initial-version row td: widen from 250 to 275 and apply the same actionCell padding-right so Start Over right-aligns with the View buttons in the rows above. Also rename the module to versionHistory.module.scss since it is now shared by both components in this table. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
MUI emotion styles inject at runtime, so they win against module CSS with equal specificity. Nest the rule under .actionCell to get specificity 0,3,0 and beat MUI's 0,2,0 `&.Mui-disabled`. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
MUI's variant selector is .MuiButton-root.MuiButton-contained .MuiButton-colorSecondary.Mui-disabled (0,4,0). Adding more selector chain still loses to that on specificity, so use !important -- this matches the existing convention in component-library button.tsx where Mui-disabled state values are also marked !important. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
CSS modules were hashing .Mui-disabled inside the selector, so the override never matched MUI's runtime class. Mark it with :global() so it stays literal. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
White text on gray-90 (#576575) = 5.96:1, comfortably AA, and reads as muted/inactive next to the vivid purple. The theme's default disabled state was white-on-light-gray (fails contrast), and no --background-* semantic token is dark enough in the light theme, so use the gray-90 primitive (allowed second-priority per the color mapping rules). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Partial migration of
apps/src/templates/VersionHistory.jsx(and its childVersionRow.jsx) for SL-1900. Scope was deliberately reduced after discovering that the full ticket scope conflicts with the DS primitives that exist today (details in Follow-ups below).What this PR does:
<button>for MUIButton(aliased asMuiButton).<h5>/<p>text for MUITypography(aliased asMuiTypography).dialog-title,caption-text,template-level-warning, the row<tr className="versionRow">, etc.).style={}props, ids (start-over-button,again-button), andonClickwiring unchanged.Button-class -> MUI mapping used:
btn-danger(Start Over / Confirm Clear)variant="contained" color="error"again-button)variant="outlined" color="secondary"btn-info(Restore on selected row, View on non-selected row)variant="contained" color="primary"img-upload(Restore on non-selected row)variant="outlined" color="primary"btn-default disabled(Latest Version, View on selected row)variant="text" color="secondary" disabledOne structural change worth calling out: the old
<a target=\"_blank\"><button class=\"btn-info\">View</button></a>was invalid HTML (button-in-anchor). It is now a single<MuiButton component=\"a\" href=... target=\"_blank\" rel=\"noopener noreferrer\">-- semantically equivalent, valid markup, same UX.No colors changed: the touched files have no inline hex / color imports, so the semantic-CSS-var part of the ticket is a no-op here.
Links
Follow-ups deferred from this ticket
These pieces of the ticket are NOT in this PR. Each needs a design call before implementation:
Wrap in DSCO
dialog—VersionHistory.jsxdoes not own its modal today. The shell comes from a legacy Bootstrap modal (createModalDialog) wrapped around it byapps/src/StudioApp.js(getVersionHistoryHandler/initVersionHistoryUI). Switching to DSCO would require the component owningisOpen/onCloseand refactoring that StudioApp.js plumbing. Also: DSCODialogmandates a singleprimaryButtonPropsrow, but the panel has three modes with different buttons -- so the right primitive isCustomDialog, notDialog. Worth confirming with design.Use DSCO
listfor version rows — DSCOSimpleListis a bulleted list of{key, label}items with a mandatory icon. The rows here are a two-column layout (timestamp + action buttons). It is a forced fit; per the design-system skill that means flagging rather than shipping.Javalab AC — the ticket says "Verified in applab and javalab", but javalab actually renders
apps/src/templates/VersionHistoryWithCommitsDialog.jsx, a different file. That file should get its own ticket.Testing story
Hey, human! Add screenshots here. I did not browser-verify this myself -- opening the legacy applab version-history modal requires an authenticated session with a saved applab/weblab project, and the local /projects/.../edit path can 500 on the AWS STS token. Please verify in a real session.
Before screenshot
After screenshot
Then, make sure you manually test the following in applab and weblab (NOT javalab -- that's a different file, see Follow-ups):
btn-default disabled(it uses theme-disabled grey instead of the oldcolor: whiteoverride). Flag if that's wrong on the dark row background.There is no "ask @designer" section for this PR -- the swap is mechanical. The genuine design questions are in the Follow-ups section above and belong to the deferred tickets.
🤖 Generated with Claude Code