microsoft / fast

The adaptive interface system for modern web experiences.
https://www.fast.design
Other
9.25k stars 590 forks source link

Fix bug in toolbar click handler when a slotted element has child elements #6830

Closed mollykreis closed 11 months ago

mollykreis commented 1 year ago

Pull Request

📖 Description

Resolves #6819

When an element in the toolbar is clicked, the toolbar's activeIndex should update to be the index of that clicked item. However, there is a bug where this does not happen if a child element within the slotted element is clicked. This PR is intended to resolve that bug.

🎫 Issues

6819

👩‍💻 Reviewer Notes

I've updated the clickHandler for the toolbar to search focusableElements for an entry that contains the clicked element rather than an entry that is the clicked element. No other logic has changed.

📑 Test Plan

I've added two additional tests for the toolbar:

  1. Ensure that the toolbar's activeIndex updates when an element slotted into the toolbar is clicked (passed with & without this change)
  2. Ensure that the toolbar's activeIndex updates when a nested element within an element slotted into the toolbar is clicked (failed without this change & passes with this change)

✅ Checklist

General

Component-specific

⏭ Next Steps

N/A

mollykreis commented 1 year ago

Could someone take a look at this PR or provide an estimate of when it might get reviewed? Thanks

@EisenbergEffect @chrisdholt @nicholasrice

mollykreis commented 1 year ago

Any update on when this PR might get some attention?

chrisdholt commented 1 year ago

Any update on when this PR might get some attention?

I'll take a look in the next day or two, thanks @mollykreis.