test(action-group): add unit, accessibility, and VRT coverage - #6589
test(action-group): add unit, accessibility, and VRT coverage#6589rise-erpelding wants to merge 9 commits into
Conversation
|
0cc1e2c to
613bade
Compare
There was a problem hiding this comment.
Compact action groups need to be able to modify the action button corner radii, this adds VRT testing for those
| const staticColorsContent = () => | ||
| ACTION_GROUP_STATIC_COLORS.map((staticColor) => | ||
| staticColorBackground( | ||
| row([renderGroup({ staticColor })], `Static ${staticColor}`), | ||
| staticColor | ||
| ) | ||
| ); |
There was a problem hiding this comment.
Is it necessary to test static colors here? We're already testing them in action button, it doesn't seem likely that anything about action group would change those test results. Debated taking it out but figured I'd gather opinions first.
There was a problem hiding this comment.
Agree to take out - if it's not implemented/managed by this component directly, then it can be removed (or added to the other, if needed).
There was a problem hiding this comment.
^ maybe a note to add to the skill to improve it/prevent this redundancy?
9a284f0 to
0737283
Compare
cdransf
left a comment
There was a problem hiding this comment.
One small question! Looks great! ✨
📚 Branch Preview Links🔍 Gen1 Visual Regression Test ResultsWhen a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:
Deployed to Azure Blob Storage: If the changes are expected, update the |
Coverage Report for CI Build 31215990171Warning No base build found for commit Coverage: 96.249%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats💛 - Coveralls |
70685fe to
5dcf4a4
Compare
5t3ph
left a comment
There was a problem hiding this comment.
Looking good, just a couple small suggestions!
| * OF ANY KIND, either express or implied. See the License for the specific language | ||
| * governing permissions and limitations under the License. | ||
| */ | ||
|
|
There was a problem hiding this comment.
What do you think of adding tests for one, two, and four buttons just to make sure the corner rounding comes through as expected? Can be one size for all.
| render: () => html` | ||
| <div | ||
| role="toolbar" | ||
| aria-label="Document actions" |
There was a problem hiding this comment.
Use of aria-orientation becomes important when role="toolbar" is applied. Maybe a note that when you do the a11y docs that you include a vertical example to show applying aria-orientation="vertical" since the default for toolbar is horizontal?
| /* @global-exclude: fills a shadow-DOM host stretched by an ancestor flex layout (e.g. vertical swc-action-group); plain elements via global classes already stretch natively */ | ||
| /* stylelint-disable-next-line no-duplicate-selectors -- re-opens .swc-ActionButton so the @global-exclude fence (see vite-global-elements-css) can strip just this declaration from the generated global stylesheet without excluding the rest of the class's rules */ | ||
| .swc-ActionButton { | ||
| inline-size: 100%; | ||
| } | ||
|
|
||
| /* @global-exclude-end */ | ||
|
|
There was a problem hiding this comment.
@5t3ph I started looking at Chromatic and realized that while <swc-action-button> isn't affected by applying inline-size: 100% (for vertical action group alignment), the global action button is. I'm excluding this style, and I committed the changes to the global stylesheet. I'm not 100% that's how this is supposed to be handled though so would appreciate some eyes on this piece.
Description
Adds Phase 6 test coverage for the 2nd-gen
swc-action-groupmigration:action-group.test.ts): defaults, ARIA attributes, roving tabindex, disabled propagation, mouse/keyboard interactions.action-group.a11y.spec.ts): ARIA snapshots, keyboard navigation (Tab in/out, arrow keys, Home/End), aXe WCAG validation, and a new ARIA-snapshot assertion for the toolbar-wrapper composition pattern.action-group.vrt.ts,action-group-custom-properties.vrt.ts): size/density/quiet permutations, justified, disabled, focus-ring z-index, static colors, forced-colors, and custom-property overrides.ToolbarCompositionstory, added to back the new toolbar-composition a11y assertion (the ARIA structure it checks has no visual signature, so it's intentionally not covered by VRT).action-button-custom-properties.vrt.ts): action-group's compact-mode corner-radius cascade added four new per-corner custom properties toswc-action-button(--swc-action-button-border-{start-start,start-end,end-start,end-end}-radius), but action-button's own custom-property VRT was never updated to cover them — its coverage assertion was failing. Added reference/override cases for all four; verified each overrides exactly one corner independently.Also updates the migration plan's testing checklist to reflect what's covered here, including a note on why toolbar-wrapper composition is verified via ARIA snapshot rather than VRT.
Motivation and context
swc-action-group's implementation and stories landed onswc-2212/migrate-action-groupwithout dedicated tests. This closes that gap ahead of the documentation pass (swc-2218/action-group-docs) and final migration review. The action-button fix was found while verifying this PR's own VRT coverage against the current custom-elements manifest.Related issue(s)
Screenshots (if appropriate)
N/A — test-only change; visual coverage is captured as Chromatic VRT snapshots.
Author's checklist
swc-2218/action-group-docsbranch.Reviewer's checklist
patch,minor, ormajorfeaturesManual review test cases
Unit/behavior suite passes
yarn testfor the 2nd-genswcpackage.action-group.test.tssuite passes with no skipped assertions.VRT stories render correctly
Permutations,ForcedColors, andCustomPropertiesfor missing rows or unstyled groups.Toolbar composition a11y assertion passes
test-2ndgen-a11y).action-group.a11y.spec.tspasses, including the new toolbar-wrapper-composition ARIA snapshot test.Action-button custom-property coverage fixed
yarn test— confirmaction-button-custom-properties.vrt.tspasses (this assertion was failing before this PR).Device review
Accessibility testing checklist
Keyboard (required — document steps below)
tabindex="0"correctly, and the second Tab leaves the group instead of moving to the next button.Screen reader (required — document steps below)
components-action-group--toolbar-composition).