Skip to content
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

fix(addon-a11y): Addon-a11y displays visual bug when running the tests #29552

Conversation

JohannesFischer
Copy link
Contributor

@JohannesFischer JohannesFischer commented Nov 6, 2024

Closes #24450

What I did

  1. Removed dynamic useResizeDetector styling from global highlight checkbox
  2. Moved GlobalToggle into List

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

  1. Run a sandbox with addon-a11y enabled
  2. Open Storybook in your browser
  3. Access a story and open the Accessibility addon panel
  4. In a story with violations (e.g. Examples / Button / Primary), click "Tests completed" button to rerun tests
  5. Verify that the highlight results checkbox no longer shifts when fetching results
  6. Reduce browser window to < 440px and verify that global checkbox wraps as expected

Mobile viewport
Screen Shot 2024-11-06 at 01 00 57

Fetching results
Screencast From 2024-11-06 01-00-04.webm

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts

  • Make sure this PR contains one of the labels below:

    Available labels
    • bug: Internal changes that fixes incorrect behavior.
    • maintenance: User-facing maintenance tasks.
    • dependencies: Upgrading (sometimes downgrading) dependencies.
    • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
    • cleanup: Minor cleanup style change. Will not show up in release changelog.
    • documentation: Documentation only changes. Will not show up in release changelog.
    • feature request: Introducing a new feature.
    • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
    • other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>

name before after diff z %
createSize 0 B 0 B 0 B - -
generateSize 78.2 MB 78.2 MB 0 B 1.38 0%
initSize 143 MB 143 MB 0 B 1.19 0%
diffSize 65.2 MB 65.2 MB 0 B 0.49 0%
buildSize 6.88 MB 6.88 MB 0 B 1.97 0%
buildSbAddonsSize 1.51 MB 1.51 MB 0 B 0.42 0%
buildSbCommonSize 195 kB 195 kB 0 B - 0%
buildSbManagerSize 1.9 MB 1.9 MB 0 B 2 0%
buildSbPreviewSize 271 kB 271 kB 0 B - 0%
buildStaticSize 0 B 0 B 0 B - -
buildPrebuildSize 3.88 MB 3.88 MB 0 B 1.97 0%
buildPreviewSize 3 MB 3 MB 0 B - 0%
testBuildSize 0 B 0 B 0 B - -
testBuildSbAddonsSize 0 B 0 B 0 B - -
testBuildSbCommonSize 0 B 0 B 0 B - -
testBuildSbManagerSize 0 B 0 B 0 B - -
testBuildSbPreviewSize 0 B 0 B 0 B - -
testBuildStaticSize 0 B 0 B 0 B - -
testBuildPrebuildSize 0 B 0 B 0 B - -
testBuildPreviewSize 0 B 0 B 0 B - -
name before after diff z %
createTime 19.4s 6.9s -12s -514ms -1.19 -180.5%
generateTime 20.5s 20.4s -99ms -0.55 -0.5%
initTime 14.5s 14.3s -219ms -0.98 -1.5%
buildTime 9s 7.7s -1s -272ms -1.01 -16.4%
testBuildTime 0ms 0ms 0ms - -
devPreviewResponsive 5.2s 5.6s 394ms -0.27 7%
devManagerResponsive 3.2s 3.3s 90ms -0.63 2.7%
devManagerHeaderVisible 479ms 510ms 31ms -0.9 6.1%
devManagerIndexVisible 512ms 575ms 63ms -0.78 11%
devStoryVisibleUncached 880ms 901ms 21ms -0.98 2.3%
devStoryVisible 511ms 573ms 62ms -0.63 10.8%
devAutodocsVisible 441ms 460ms 19ms -0.82 4.1%
devMDXVisible 452ms 492ms 40ms -0.34 8.1%
buildManagerHeaderVisible 501ms 509ms 8ms -0.82 1.6%
buildManagerIndexVisible 518ms 521ms 3ms -0.86 0.6%
buildStoryVisible 510ms 504ms -6ms -0.87 -1.2%
buildAutodocsVisible 483ms 412ms -71ms -0.95 -17.2%
buildMDXVisible 382ms 408ms 26ms -0.65 6.4%

Greptile Summary

Fixed visual bug in addon-a11y's highlight results checkbox by simplifying the GlobalToggle component layout and improving responsiveness.

  • Removed dynamic width-based styling from GlobalToggle in /code/addons/a11y/src/components/Tabs.tsx
  • Added flexWrap to List component to handle mobile viewport layouts
  • Moved GlobalToggle inside List component for better positioning
  • Simplified margins and padding on GlobalToggle to prevent shifting during test runs

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

Copy link

nx-cloud bot commented Nov 6, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 88a1ad2. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@JohannesFischer
Copy link
Contributor Author

Have you had a chance to review the changes @yannbf? Any thoughts?

@valentinpalkovic valentinpalkovic changed the base branch from next to valentin/unified-a11y-testing November 25, 2024 08:24
@valentinpalkovic valentinpalkovic merged commit aae1bac into storybookjs:valentin/unified-a11y-testing Nov 25, 2024
51 of 55 checks passed
@JohannesFischer JohannesFischer deleted the 24450-addon-a11y-highlight-visual-bug branch November 27, 2024 01:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Addon-a11y displays visual bug when running the tests
4 participants