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(fast-element1): fix toolbar navigation tests #6976

Merged

Conversation

m-akinc
Copy link
Contributor

@m-akinc m-akinc commented Jun 5, 2024

Pull Request

📖 Description

Ten of the fast-foundation toolbar tests (related to focus) began failing seemingly due to browser changes. The tests fail in Chrome as well as Firefox. It seems that calling focus() on the toolbar element does not pass the focus on to a contained element. I've worked around this for nine of the tests by directly focusing a button initially, instead of focusing the toolbar and expecting the button to get focus. The tenth failing test is testing exactly the behavior that stopped working. I believe we will have to just disable that test.

Also, removing a stray .only that was apparently checked in accidentally.

🎫 Issues

👩‍💻 Reviewer Notes

📑 Test Plan

✅ Checklist

General

  • I have included a change request file using $ yarn change
  • I have added tests for my changes.
  • I have tested my changes.
  • I have updated the project documentation to reflect my changes.
  • I have read the CONTRIBUTING documentation and followed the standards for this project.

⏭ Next Steps

@m-akinc
Copy link
Contributor Author

m-akinc commented Jun 6, 2024

I would imagine these changes are pretty non-controversial, especially since they fix tests that currently do not pass in this branch. @janechu are you comfortable merging this without @chrisdholt's review?

@chrisdholt chrisdholt merged commit 1f9c97d into microsoft:archives/fast-element-1 Jun 6, 2024
5 checks passed
@m-akinc m-akinc deleted the fix-toolbar-tests branch June 6, 2024 23:02
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.

3 participants