Merge pull request #73 from github/data_no_focus_bug

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

Author: keithamus

Diff for: src/index.ts

@@ -320,20 +320,43 @@ function applyFromToolbar(event: Event) {
   applyStyle(target, style)
 }
 
+function setFocusManagement(toolbar: MarkdownToolbarElement) {
+  toolbar.addEventListener('keydown', focusKeydown)
+  toolbar.setAttribute('tabindex', '0')
+  toolbar.addEventListener('focus', onToolbarFocus, {once: true})
+}
+
+function unsetFocusManagement(toolbar: MarkdownToolbarElement) {
+  toolbar.removeEventListener('keydown', focusKeydown)
+  toolbar.removeAttribute('tabindex')
+  toolbar.removeEventListener('focus', onToolbarFocus)
+}
+
 class MarkdownToolbarElement extends HTMLElement {
+  static observedAttributes = ['data-no-focus']
+
   connectedCallback(): void {
     if (!this.hasAttribute('role')) {
       this.setAttribute('role', 'toolbar')
     }
-    this.addEventListener('keydown', focusKeydown)
-    this.setAttribute('tabindex', '0')
-    this.addEventListener('focus', onToolbarFocus, {once: true})
+    if (!this.hasAttribute('data-no-focus')) {
+      setFocusManagement(this)
+    }
     this.addEventListener('keydown', keydown(applyFromToolbar))
     this.addEventListener('click', applyFromToolbar)
   }
 
+  attributeChangedCallback(name: string, oldValue: string, newValue: string): void {
+    if (name !== 'data-no-focus') return
+    if (newValue === null) {
+      setFocusManagement(this)
+    } else {
+      unsetFocusManagement(this)
+    }
+  }
+
   disconnectedCallback(): void {
-    this.removeEventListener('keydown', focusKeydown)
+    unsetFocusManagement(this)
   }
 
   get field(): HTMLTextAreaElement | null {
@@ -350,7 +373,6 @@ class MarkdownToolbarElement extends HTMLElement {
 
 function onToolbarFocus({target}: FocusEvent) {
   if (!(target instanceof Element)) return
-  if (target.hasAttribute('data-no-focus')) return
   target.removeAttribute('tabindex')
   let tabindex = '0'
   for (const button of getButtons(target)) {
@@ -367,7 +389,6 @@ function focusKeydown(event: KeyboardEvent) {
   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
   const buttons = getButtons(toolbar)
   const index = buttons.indexOf(event.target as HTMLElement)
   const length = buttons.length

Diff for: test/test.js

@@ -96,6 +96,7 @@ describe('markdown-toolbar-element', function () {
 
     describe('focus management', function () {
       function focusFirstButton() {
+        document.querySelector('markdown-toolbar').focus()
         const button = document.querySelector('md-bold')
         button.focus()
       }
@@ -111,10 +112,6 @@ describe('markdown-toolbar-element', function () {
         return [...document.querySelectorAll(`markdown-toolbar [tabindex="${index}"]`)]
       }
 
-      beforeEach(() => {
-        document.querySelector('markdown-toolbar').focus()
-      })
-
       it('moves focus to next button when ArrowRight is pressed', function () {
         focusFirstButton()
         pushKeyOnFocussedButton('ArrowRight')
@@ -135,8 +132,7 @@ describe('markdown-toolbar-element', function () {
         document.querySelector('markdown-toolbar').setAttribute('data-no-focus', '')
         focusFirstButton()
         pushKeyOnFocussedButton('ArrowRight')
-        assert.deepEqual(getElementsWithTabindex(0), [document.querySelector('md-bold')])
-        assert.deepEqual(getElementsWithTabindex(0), [document.activeElement])
+        assert.lengthOf(getElementsWithTabindex(0), 0)
       })
 
       it('cycles focus round to last element from first when ArrowLeft is pressed', function () {