va-modal: Update spacing and button layout#1984
Conversation
babsdenney
left a comment
There was a problem hiding this comment.
Such a big improvement! Very exciting!
RyanMunsch
left a comment
There was a problem hiding this comment.
Good stuff, @danbrady! Just this one small thing on my end but this is looking good to me!
62feae1 to
29d70ab
Compare
RyanMunsch
left a comment
There was a problem hiding this comment.
LGTM. The component looks much cleaner now 😎
|
@amyleadem Ready for your re-review! 🙏 |
amyleadem
left a comment
There was a problem hiding this comment.
Code changes are looking good! Just a couple open design questions/issues.
There was a problem hiding this comment.
Confirming that all of the open comments have been addressed. Nice work!
However, as a final check I took a look at the components that use va-modal, and found this vertical spacing inconsistency in va-file-input:
Previously the footer used the .usa-modal__footer class, which had a top margin of 3rem. Can that (or something similar) be restored here?
Note that I also checked va-crisis-line-modal, va-omb-info in Chrome and desktop and mobile widths and they looked good.
|
Amy is officially in the running for a 🦅👁️🏆! I restored the USWDS class that adds |
amyleadem
left a comment
There was a problem hiding this comment.
LGTM!
I found one possible style issue that I am considering optional, but the bottom padding on the large variant seems to be excessive and does not match the default variant. This was there in previous reviews, but the increase in the top margin of the footer just made it more pronounced. If this wasn't intentional, I recommend we reduce that vertical space.
|
@amyleadem Ah, ok. The USWDS styles were injected that extra spacing in. I raised the specificity of our |
Chromatic
https://db-update-modal-layout-5043--65a6e2ed2314f7b8f98609d8.chromatic.com
Description
This PR refactors the va-modal web component with a focus on simplifying and modernizing its styling. The updates improve presentation, accessibility, and responsiveness.
Key updates include better spacing on mobile to reduce text wrapping, especially for long button labels, smaller icons with less surrounding whitespace to free up content space, and full-width buttons to keep visual weight consistent regardless of label length.
It also cleans up the codebase by removing deprecated or unused styles, props, and markup like legacy alert actions and the unused unstyled prop. Formatting and structure in the TypeScript file were also improved to make the code more consistent and easier to read.
Related tickets and links
Closes #5043
Screenshots
Default Modal / Mobile
Default Modal / Desktop
Status Modal / Mobile
Status Modal / Desktop
Testing and review
Approvals
See the QA Checklists section below for suggested approvals. Use your best judgment if additional reviews are needed. When in doubt, request a review.
Approval groups
Add approval groups to the PR as needed:
QA checklists
Use the QA checklists below as guides, not rules. Not all checklists will apply to every PR but there could be some overlap.
In all scenarios, changes should be fully tested by the author and verified by the reviewer(s); functionality, responsiveness, etc.
✨ New Component Added
minorlabel🌱 New Component Variation Added
minorlabel🐞 Component Fix
patchlabel♿️ Component Fix - Accessibility
patchlabel🚨 Component Fix - Breaking API Change
majorlabel🔧 Component Update - Non-Breaking API Change
minorlabel📖 Storybook Update
ignore-for-releaselabel🎨 CSS-Library Update
css-librarylabel