Skip to content

Don't set a tabindex=0 when the toolbar has data-no-focus attribute applied#73

Merged
keithamus merged 5 commits into
mainfrom
data_no_focus_bug
Sep 22, 2023
Merged

Don't set a tabindex=0 when the toolbar has data-no-focus attribute applied#73
keithamus merged 5 commits into
mainfrom
data_no_focus_bug

Conversation

@jonrohan

@jonrohan jonrohan commented Sep 7, 2023

Copy link
Copy Markdown
Member

The markdown-toolbar element is grabbing focus when using data-no-focus. I think this is because of the tabindex that's being applied.

image

To fix this, I'm moving the this.hasAttribute('data-no-focus') into the connectedCallback constructor and also avoiding adding eventListeners when this occurs

@jonrohan jonrohan requested a review from a team as a code owner September 7, 2023 22:10
@jonrohan jonrohan requested a review from keithamus September 7, 2023 22:10
@primer-css

Copy link
Copy Markdown

👋 Hello and thanks for pinging us! This issue or PR has been added to our inbox and a Primer first responder will review it soon.

  • 🎨 If this is a PR that includes a visual change, please make sure to add screenshots in the description or deploy this code to a lab machine with instructions for how to test.
  • If this is a PR that includes changes to an interaction, please include a video recording in the description.
  • ⚠️ If this is urgent, please visit us in #primer on Slack and tag the first responders listed in the channel topic.
Comment thread src/index.ts
if (key !== 'ArrowRight' && key !== 'ArrowLeft' && key !== 'Home' && key !== 'End') return
const toolbar = event.currentTarget
if (!(toolbar instanceof HTMLElement)) return
if (toolbar.hasAttribute('data-no-focus')) return

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this line need to be removed? I see it prevents arrow keys from functioning when the toolbar has the attribute.

@keithamus keithamus merged commit 666d0c2 into main Sep 22, 2023
@keithamus keithamus deleted the data_no_focus_bug branch September 22, 2023 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

7 participants