mollykreis commented 1 year ago

Pull Request

📖 Description

Resolves #6835

There is a bug where when clicking on an element in the toolbar focuses the activeIndex element before focusing the clicked element. This can lead to visual oddities. This PR is intended to resolve that bug.

🎫 Issues

6835

👩‍💻 Reviewer Notes

I changed the toolbar to use the mousedown event rather than the click event to update the toolbar's activeIndex because the mousedown event is fired before focusin, thus resolving the timing issue where focusin focused the previously focused element prior to click focusing the newly focused element.

📑 Test Plan

I wasn't able to write a test that failed prior to my change but passes with it because the issue was a temporary state that was passed through during a click operations. However, I wrote two new tests that exercise the modified code path.

I also manually tested my change in Chrome, Edge, and Firefox.

✅ Checklist

General

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

Component-specific

[ ] I have added a new component
[x] I have modified an existing component
[ ] I have updated the definition file
[ ] I have updated the configuration file

⏭ Next Steps

N/A

mollykreis commented 11 months ago

Could I get a second review on this PR? @EisenbergEffect @chrisdholt @nicholasrice

chrisdholt commented 11 months ago