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

Wrong toolbar button getting focused when the toolbar receives focus #1423

Closed
vivinkrishna-ni opened this issue Aug 10, 2023 · 4 comments · Fixed by #1642
Closed

Wrong toolbar button getting focused when the toolbar receives focus #1423

vivinkrishna-ni opened this issue Aug 10, 2023 · 4 comments · Fixed by #1642
Assignees
Labels
bug Something isn't working

Comments

@vivinkrishna-ni
Copy link
Contributor

🐛 Bug Report

In nimble-toolbar, button focusing operates based on the tabindex value. However, when clicking (mouse down) on another button with tabindex=-1, the button with tabindex=0 receives focus during the mouse down event. This makes the nimble-button being focused when clicking another button.

Note: This issue is reproducible only in the Chrome browser.

💻 Repro or Code Sample

  1. Adding another control for the nimble-toolbar (in the below example added textarea)
  2. In the click event, add a focus event to the other control (say textarea)
  3. Click the button that is not recently focused (i.e. not the one with tabindex=0)

🤔 Expected Behavior

No button in the toolbar should receive focus.

😯 Current Behavior

The button with the tabindex=0 receives focus.
259184551-24c12137-3fce-43a8-a96f-3b4af0ad6a3f

💁 Possible Solution

🔦 Context

It is needed in the nimble-rich-text-editor as we focus back on the editor when a formatting button in the nimble-toolbar is clicked.
For more details, see #1416 (comment) (Issue 1)

🌍 Your Environment

  • OS & Device: Windows on PC
  • Browser Google Chrome
  • Version 115.0.5790.170
@vivinkrishna-ni vivinkrishna-ni added bug Something isn't working triage New issue that needs to be reviewed labels Aug 10, 2023
@mollykreis
Copy link
Contributor

The issue appears to be in the FAST toolbar, as I can reproduce the problem in their toolbar as well. The toolbar's clickHandler changes the newly clicked item to be the focused item, but this handler doesn't run until the click event occurs (which is on mouse up). However, the toolbar's focusinHandler re-focus what was previously focused when the toolbar receives focus. The focusinHandler runs on mouse down, so the previously focused item gets the focus-visible styling while the mouse is pressed, and then the clickHandler adjusts the focused element when the mouse is released.

@m-akinc m-akinc removed the triage New issue that needs to be reviewed label Aug 15, 2023
@m-akinc m-akinc moved this to Current Iteration in Nimble Design System Priorities Aug 15, 2023
@mollykreis
Copy link
Contributor

mollykreis commented Aug 22, 2023

I created microsoft/fast#6819 to track the underlying issue in the FAST toolbar.

However, we should also evaluate whether adding pointer-events: none to some of our nimble styles makes sense, such as the start and end slots of our button styles.

Edit: I added this comment to the wrong issue. This comment doesn't apply to this issue.

@mollykreis
Copy link
Contributor

I created microsoft/fast#6835 to track the underlying issue in the FAST toolbar.

@jattasNI
Copy link
Contributor

jattasNI commented Oct 5, 2023

Marking as blocked to reflect the dependency on the FAST issue

@jattasNI jattasNI added the blocked Blocked on a third-party issue label Oct 5, 2023
@mollykreis mollykreis removed the blocked Blocked on a third-party issue label Nov 1, 2023
mollykreis added a commit that referenced this issue Nov 2, 2023
# Pull Request

## 🤨 Rationale

The latest version of fast-foundation has fixes for two of our toolbar
issues, so this PR pulls in the latest version.

Resolves #1422 
Resolves #1423 

## 👩‍💻 Implementation

Bump version in package file
Update the rich-text-editor page object to adjust for a change in the
toolbar code

## 🧪 Testing

Used storybook to verify that the two toolbar issues are resolved

## ✅ Checklist

<!--- Review the list and put an x in the boxes that apply or ~~strike
through~~ around items that don't (along with an explanation). -->

- [ ] I have updated the project documentation to reflect my changes or
determined no changes are needed.
@m-akinc m-akinc moved this from In progress to Done in Nimble Design System Priorities Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Development

Successfully merging a pull request may close this issue.

4 participants