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

Add dialog role and tests #4739

Closed
wants to merge 2 commits into from

Conversation

jabsatz
Copy link

@jabsatz jabsatz commented May 27, 2021

Recreating pull #4734 so it correctly builds in CircleCI

Fixes #4528

Checklist

  • Includes tests

  • Update documentation

Changes proposed in this pull request:

Add dialog role to the Dialog component.

Reviewers should focus on:

Correctness of the test, since the change itself pretty simple.

Screenshot

@jabsatz
Copy link
Author

jabsatz commented May 27, 2021

Looks like some tests are effectively breaking with this change. I wasn't able to test the project properly yesterday because of some environment issues on my home PC. I was hoping since the change was small they would pass directly in CircleCI, but unfortunately I didn't have such luck.
I will try again later today from another computer and see if I can build it and test it correctly before pushing another fix.

@adidahiya
Copy link
Contributor

Closing for now, happy to re-open if you push a new commit

@adidahiya adidahiya closed this Jun 22, 2021
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.

Dialogs should have proper a11y markup
2 participants