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 wcag github action mapbox issues #337

Merged
merged 4 commits into from
Nov 17, 2022
Merged

Fix wcag github action mapbox issues #337

merged 4 commits into from
Nov 17, 2022

Conversation

yen-tt
Copy link
Contributor

@yen-tt yen-tt commented Nov 16, 2022

paired with the changes in this PR in WCAG workflow, this PR updates the WCAG github action in the repo to pass in the mapbox key. Also updated the wcag test to exclude checking elements coming from mapbox canvas container as any potential violations coming from there is outside of our repo's control.

WCAG github action also run on pull request to feature branch now.

J=SLAP-2458
TEST=auto

see that WCAG github action now passes

@yen-tt yen-tt added the wip label Nov 16, 2022
@yen-tt yen-tt requested a review from a team as a code owner November 16, 2022 19:49
@coveralls
Copy link

coveralls commented Nov 16, 2022

Coverage Status

Coverage remained the same at 84.395% when pulling 028161d on dev/wcag-fix into a2e8319 on develop.

@yen-tt yen-tt changed the title test Fix wcag github action mapbox issues Nov 16, 2022
@yen-tt yen-tt removed the wip label Nov 16, 2022
Copy link
Contributor

@nmanu1 nmanu1 left a comment

Choose a reason for hiding this comment

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

do you know why the coverage check is still failing even though the WCAG check passes? I thought they were related

page,
{
include: ['#root'],
exclude: ['#root .mapboxgl-canvas-container'],
Copy link
Collaborator

Choose a reason for hiding this comment

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

For my own edification, was this line the only change needed to get the WCAG tests passing? If we're ignoring Mapbox, the passing of the MAPBOX_API_KEY shouldn't really matter, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both the passing of mapbox api key and the change in test-runner is needed to get WCAG test passing. It couldn't properly run checks on mapbox stories because the mapbox component never gets render due to no api key, and that causes errors within play function and closed the browser/session. With MAPBOX_API_KEY, the mapbox stories was evaluated correctly, but there are a couple violations within mapbox canvas, which is what the change in test-runner is for.

@yen-tt
Copy link
Contributor Author

yen-tt commented Nov 17, 2022

do you know why the coverage check is still failing even though the WCAG check passes? I thought they were related

I did a brief investigation yesterday and it appears to be a separate issue, where the visual coverage script seems to be killing the process after running test-storybook --coverage and didn't get to the rest of the commands for generating lcov coverage and moving the files to the expected directory. That will be another item / PR.

@tmeyer2115 tmeyer2115 self-requested a review November 17, 2022 14:28
@yen-tt yen-tt merged commit ee60b9f into develop Nov 17, 2022
@yen-tt yen-tt deleted the dev/wcag-fix branch November 17, 2022 14:38
yen-tt added a commit that referenced this pull request Nov 18, 2022
paired with the changes in this [PR](yext/slapshot-reusable-workflows#22) in WCAG workflow, this PR updates the WCAG github action in the repo to pass in the mapbox key. Also updated the wcag test to exclude checking elements coming from mapbox canvas container as any potential violations coming from there is outside of our repo's control.

WCAG github action also run on pull request to feature branch now.

J=SLAP-2458
TEST=auto

see that WCAG github action now passes
nmanu1 pushed a commit that referenced this pull request Dec 2, 2022
paired with the changes in this [PR](yext/slapshot-reusable-workflows#22) in WCAG workflow, this PR updates the WCAG github action in the repo to pass in the mapbox key. Also updated the wcag test to exclude checking elements coming from mapbox canvas container as any potential violations coming from there is outside of our repo's control.

WCAG github action also run on pull request to feature branch now.

J=SLAP-2458
TEST=auto

see that WCAG github action now passes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants