GAUD-10289 - Add initial divider axe, unit and vdiff tests#7217
Conversation
|
Thanks for the PR! 🎉 We've deployed an automatic preview for this PR - you can see your changes here:
Note The build needs to finish before your changes are deployed. |
| }); | ||
| }); | ||
|
|
||
| describe('focus', () => { |
There was a problem hiding this comment.
The variety of focus and focus-scrolled vdiffs is kinda pointless right now, but will eventually be important to show where the collapse button and floating arrows live in the different scenarios.
| [ | ||
| { name: 'side-nav', divider: 'side-nav', fixture: pageDividerFixtures.sideNavBothHeadersFooter }, | ||
| { name: 'side-nav-immersive', divider: 'side-nav', fixture: pageDividerFixtures.sideNavImmersiveFooter }, | ||
| { name: 'side-nav-long-no-scroll', divider: 'side-nav', fixture: pageDividerFixtures.sideNavLongMainBothHeaders }, |
There was a problem hiding this comment.
page-divider-internal focus side-nav-long-no-scroll, page-divider-internal focus supporting, and page-divider-internal focus supporting-immersive-long-no-scroll are currently scrolled when they shouldn't be. This is because of https://desire2learn.atlassian.net/browse/GAUD-10291, and will be fixed in the next PR.
| const currentSize = 450; | ||
| const minSize = 320; | ||
| const maxSize = 600; | ||
| const step = 20; |
There was a problem hiding this comment.
Wonder if it's worth exporting these constants from the component so they can be shared?
There was a problem hiding this comment.
Yeah I meant to ask about that - it'll come up in future tests too. I wasn't sure if we changed the step size if we'd want these tests to fail to force us to take a look, rather than self updating? But I'm not really sure what that buys us, the tests are already written in a way to adjust accordingly. I went back and forth on it, I'll update.
| it(test.name, async() => { | ||
| const elem = await fixture(test.fixture, { pagePadding: false, viewport: { width: 1000, height: 550 } }); | ||
| await hoverElem(getDivider(elem, test.divider)); | ||
| await expect(elem).to.be.golden({ margin: 0 }); |
There was a problem hiding this comment.
I guess it's necessary for all of these to screenshot the whole page vs. just the divider? Just wondering if we could save some time.
There was a problem hiding this comment.
Yeah I thought about it, but most of the visual part of the divider worth testing is how it interacts with the page. hover could probably move out, but that's assuming we don't end up adding any additional elements to help with dragging/selecting on hover.
I tried to make sure there's not duplication - page vdiffs has no focus/hover tests for divider, and divider vdiffs don't test the default state (which is covered by page vdiffs).
I could probably make them shorter though! And a tad less wide, but not much.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
🎉 This PR is included in version 3.271.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
2 participants
Many more to come as the divider is fleshed out.
vdiff report.