Title: [257411] trunk/Source/WebInspectorUI
Revision
257411
Author
nvasil...@apple.com
Date
2020-02-25 23:18:43 -0800 (Tue, 25 Feb 2020)

Log Message

Web Inspector: AXI: buttons should be focusable when navigating by pressing Tab
https://bugs.webkit.org/show_bug.cgi?id=208163
<rdar://problem/59745448>

Reviewed by Brian Burg.

Buttons now accessible with Tab navigation. The focused button has the native focus outline.

Clicking on the button does NOT move the focus. For example, when you're focused on the
console prompt, clicking on the icon to hide the right sidebar keeps you focused where
you were — the console prompt. This behavior matches macOS.

Convert WI.NavigationItem with role=button from `<div>` to `<button>` elements.
Button elements have implicit tabIndex=0. When focused, pressing Space or Enter
triggers "click" event.

* UserInterface/Views/ActivateButtonNavigationItem.js:
(WI.ActivateButtonNavigationItem.prototype.set activated):
Add "aria-pressed" and "aria-label" attributes for VoiceOver.

* UserInterface/Views/ButtonNavigationItem.css:
(.navigation-bar .item.button:not(.image-only):focus,):
(.navigation-bar .item.button.image-only:focus):
(.navigation-bar .item.button:not(.disabled):matches(.activate.activated, .radio.selected) > .glyph):
(.navigation-bar .item.button:not(.disabled):active:matches(.activate.activated, .radio.selected) > .glyph):
Before this patch, focused button looked the same as activated buttons.
For example, the focused (non-active) bullseye icon looked exactly the
same as unfocused active bullseye icon, which was misleading.

* UserInterface/Views/ButtonNavigationItem.js:
(WI.ButtonNavigationItem):
(WI.ButtonNavigationItem.prototype._mouseClicked):
(WI.ButtonNavigationItem.prototype._handleMouseDown):
Clicking on a button shouldn't move focus. For example, when you're focused on the console prompt,
clicking on the icon to hide the right sidebar should keep you focused where you were - the console prompt.

(WI.ButtonNavigationItem.prototype._handleKeyDown):
* UserInterface/Views/ButtonToolbarItem.css:
(.toolbar .item.button):
Adjust outline offset to remove the gap between the outline and the border of the button.

(.toolbar .item.button:not(.disabled).activate.activated):
(@media (prefers-color-scheme: dark) body:not(.window-inactive) .toolbar .item.button:not(.disabled).activate.activated > .glyph):
* UserInterface/Views/ControlToolbarItem.css:
(.toolbar .item.control:focus):
(.toolbar .item.control:focus > .glyph):
Draw the outline around the X (close button) glyph, not the button itself (which is much wider than the glyph).

* UserInterface/Views/NavigationBar.css:
(.navigation-bar .item):
(.navigation-bar .item:focus):
* UserInterface/Views/NavigationBar.js:
(WI.NavigationBar):
"focus" and "blur" events don't bubble. These event handlers didn't capture the event.

(WI.NavigationBar.prototype._mouseDown):
(WI.NavigationBar.prototype._keyDown):
* UserInterface/Views/NavigationItem.js:
(WI.NavigationItem):
* UserInterface/Views/RadioButtonNavigationItem.css:
(.navigation-bar .item.radio.button:focus):
* UserInterface/Views/Toolbar.css:
(.toolbar .item):

Modified Paths

Diff

Modified: trunk/Source/WebInspectorUI/ChangeLog (257410 => 257411)


--- trunk/Source/WebInspectorUI/ChangeLog	2020-02-26 06:05:13 UTC (rev 257410)
+++ trunk/Source/WebInspectorUI/ChangeLog	2020-02-26 07:18:43 UTC (rev 257411)
@@ -1,3 +1,69 @@
+2020-02-25  Nikita Vasilyev  <nvasil...@apple.com>
+
+        Web Inspector: AXI: buttons should be focusable when navigating by pressing Tab
+        https://bugs.webkit.org/show_bug.cgi?id=208163
+        <rdar://problem/59745448>
+
+        Reviewed by Brian Burg.
+
+        Buttons now accessible with Tab navigation. The focused button has the native focus outline.
+
+        Clicking on the button does NOT move the focus. For example, when you're focused on the
+        console prompt, clicking on the icon to hide the right sidebar keeps you focused where
+        you were — the console prompt. This behavior matches macOS.
+
+        Convert WI.NavigationItem with role=button from `<div>` to `<button>` elements.
+        Button elements have implicit tabIndex=0. When focused, pressing Space or Enter
+        triggers "click" event.
+
+        * UserInterface/Views/ActivateButtonNavigationItem.js:
+        (WI.ActivateButtonNavigationItem.prototype.set activated):
+        Add "aria-pressed" and "aria-label" attributes for VoiceOver.
+
+        * UserInterface/Views/ButtonNavigationItem.css:
+        (.navigation-bar .item.button:not(.image-only):focus,):
+        (.navigation-bar .item.button.image-only:focus):
+        (.navigation-bar .item.button:not(.disabled):matches(.activate.activated, .radio.selected) > .glyph):
+        (.navigation-bar .item.button:not(.disabled):active:matches(.activate.activated, .radio.selected) > .glyph):
+        Before this patch, focused button looked the same as activated buttons.
+        For example, the focused (non-active) bullseye icon looked exactly the
+        same as unfocused active bullseye icon, which was misleading.
+
+        * UserInterface/Views/ButtonNavigationItem.js:
+        (WI.ButtonNavigationItem):
+        (WI.ButtonNavigationItem.prototype._mouseClicked):
+        (WI.ButtonNavigationItem.prototype._handleMouseDown):
+        Clicking on a button shouldn't move focus. For example, when you're focused on the console prompt,
+        clicking on the icon to hide the right sidebar should keep you focused where you were - the console prompt.
+
+        (WI.ButtonNavigationItem.prototype._handleKeyDown):
+        * UserInterface/Views/ButtonToolbarItem.css:
+        (.toolbar .item.button):
+        Adjust outline offset to remove the gap between the outline and the border of the button.
+
+        (.toolbar .item.button:not(.disabled).activate.activated):
+        (@media (prefers-color-scheme: dark) body:not(.window-inactive) .toolbar .item.button:not(.disabled).activate.activated > .glyph):
+        * UserInterface/Views/ControlToolbarItem.css:
+        (.toolbar .item.control:focus):
+        (.toolbar .item.control:focus > .glyph):
+        Draw the outline around the X (close button) glyph, not the button itself (which is much wider than the glyph).
+
+        * UserInterface/Views/NavigationBar.css:
+        (.navigation-bar .item):
+        (.navigation-bar .item:focus):
+        * UserInterface/Views/NavigationBar.js:
+        (WI.NavigationBar):
+        "focus" and "blur" events don't bubble. These event handlers didn't capture the event.
+
+        (WI.NavigationBar.prototype._mouseDown):
+        (WI.NavigationBar.prototype._keyDown):
+        * UserInterface/Views/NavigationItem.js:
+        (WI.NavigationItem):
+        * UserInterface/Views/RadioButtonNavigationItem.css:
+        (.navigation-bar .item.radio.button:focus):
+        * UserInterface/Views/Toolbar.css:
+        (.toolbar .item):
+
 2020-02-25  Devin Rousso  <drou...@apple.com>
 
         Web Inspector: safari app extension isolated worlds and injected files use the extension's identifier instead of its name

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/ActivateButtonNavigationItem.js (257410 => 257411)


--- trunk/Source/WebInspectorUI/UserInterface/Views/ActivateButtonNavigationItem.js	2020-02-26 06:05:13 UTC (rev 257410)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/ActivateButtonNavigationItem.js	2020-02-26 07:18:43 UTC (rev 257411)
@@ -69,17 +69,13 @@
 
     set activated(flag)
     {
+        flag = !!flag;
         this.element.classList.toggle(WI.ActivateButtonNavigationItem.ActivatedStyleClassName, flag);
 
-        if (flag) {
-            this.tooltip = this._activatedToolTip;
-            if (this._role === "tab")
-                this.element.setAttribute("aria-selected", "true");
-        } else {
-            this.tooltip = this._defaultToolTip;
-            if (this._role === "tab")
-                this.element.removeAttribute("aria-selected");
-        }
+        this.tooltip = flag ? this._activatedToolTip : this._defaultToolTip;
+
+        this.element.ariaPressed = flag;
+        this.element.ariaLabel = this.tooltip;
     }
 
     // Protected

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/ButtonNavigationItem.css (257410 => 257411)


--- trunk/Source/WebInspectorUI/UserInterface/Views/ButtonNavigationItem.css	2020-02-26 06:05:13 UTC (rev 257410)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/ButtonNavigationItem.css	2020-02-26 07:18:43 UTC (rev 257411)
@@ -28,6 +28,15 @@
     color: hsl(0, 0%, 18%);
 }
 
+.navigation-bar .item.button:not(.image-only):focus,
+.navigation-bar .item.button.image-only:focus > .glyph {
+    outline: auto -webkit-focus-ring-color;
+}
+
+.navigation-bar .item.button.image-only:focus {
+    outline: none;
+}
+
 .navigation-bar .item.button.image-only {
     width: 26px;
 }
@@ -61,11 +70,11 @@
     color: var(--glyph-color-pressed);
 }
 
-.navigation-bar .item.button:not(.disabled):matches(:focus, .activate.activated, .radio.selected) > .glyph {
+.navigation-bar .item.button:not(.disabled):matches(.activate.activated, .radio.selected) > .glyph {
     color: var(--glyph-color-active);
 }
 
-.navigation-bar .item.button:not(.disabled):active:matches(:focus, .activate.activated, .radio.selected) > .glyph {
+.navigation-bar .item.button:not(.disabled):active:matches(.activate.activated, .radio.selected) > .glyph {
     color: var(--glyph-color-active-pressed);
 }
 

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/ButtonNavigationItem.js (257410 => 257411)


--- trunk/Source/WebInspectorUI/UserInterface/Views/ButtonNavigationItem.js	2020-02-26 06:05:13 UTC (rev 257410)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/ButtonNavigationItem.js	2020-02-26 07:18:43 UTC (rev 257411)
@@ -27,7 +27,8 @@
 {
     constructor(identifier, toolTipOrLabel, image, imageWidth, imageHeight, role, label)
     {
-        super(identifier, role || "button");
+        role = role || "button";
+        super(identifier, role);
 
         console.assert(identifier);
         console.assert(toolTipOrLabel);
@@ -36,6 +37,14 @@
 
         this.element.addEventListener("click", this._mouseClicked.bind(this));
 
+        // Don't move the focus on the button when clicking on it. This matches macOS behavior.
+        this.element.addEventListener("mousedown", this._handleMouseDown.bind(this), true);
+
+        if (role === "button") {
+            this.element.tabIndex = 0;
+            this.element.addEventListener("keydown", this._handleKeyDown.bind(this));
+        }
+
         if (label)
             this.element.setAttribute("aria-label", label);
 
@@ -142,6 +151,25 @@
 
     _mouseClicked(event)
     {
+        this._buttonPressed(event);
+    }
+
+    _handleMouseDown(event)
+    {
+        // Clicking on a button should NOT focus on it.
+        event.stop();
+    }
+
+    _handleKeyDown(event)
+    {
+        if (event.code === "Enter" || event.code === "Space") {
+            event.stop();
+            this._buttonPressed(event);
+        }
+    }
+
+    _buttonPressed(event)
+    {
         if (!this.enabled)
             return;
         this.dispatchEventToListeners(WI.ButtonNavigationItem.Event.Clicked, {nativeEvent: event});

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/ButtonToolbarItem.css (257410 => 257411)


--- trunk/Source/WebInspectorUI/UserInterface/Views/ButtonToolbarItem.css	2020-02-26 06:05:13 UTC (rev 257410)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/ButtonToolbarItem.css	2020-02-26 07:18:43 UTC (rev 257411)
@@ -31,6 +31,7 @@
     padding: 0 10px;
 
     color: hsl(0, 0%, 45%);
+    outline-offset: -2px;
 }
 
 .toolbar .item.button:not(.disabled):active {
@@ -37,7 +38,7 @@
     color: hsl(0, 0%, 30%);
 }
 
-.toolbar .item.button:not(.disabled):matches(:focus, .activate.activated) {
+.toolbar .item.button:not(.disabled).activate.activated {
     color: var(--glyph-color-active);
 }
 
@@ -69,7 +70,7 @@
         background: var(--button-background-color-pressed);
     }
 
-    body:not(.window-inactive) .toolbar .item.button:not(.disabled):matches(:focus, .activate.activated) > .glyph {
+    body:not(.window-inactive) .toolbar .item.button:not(.disabled).activate.activated > .glyph {
         filter: brightness(1.35);
     }
 }

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/ControlToolbarItem.css (257410 => 257411)


--- trunk/Source/WebInspectorUI/UserInterface/Views/ControlToolbarItem.css	2020-02-26 06:05:13 UTC (rev 257410)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/ControlToolbarItem.css	2020-02-26 07:18:43 UTC (rev 257411)
@@ -52,6 +52,14 @@
     background-position: center center;
 }
 
+.toolbar .item.control:focus {
+    outline: none;
+}
+
+.toolbar .item.control:focus > .glyph {
+    outline: auto -webkit-focus-ring-color;
+}
+
 body.window-inactive .toolbar .item.control {
     opacity: 0.35;
 }

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/NavigationBar.css (257410 => 257411)


--- trunk/Source/WebInspectorUI/UserInterface/Views/NavigationBar.css	2020-02-26 06:05:13 UTC (rev 257410)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/NavigationBar.css	2020-02-26 07:18:43 UTC (rev 257411)
@@ -46,9 +46,13 @@
     flex-wrap: wrap;
 
     height: auto;
-    outline: none;
+    outline-offset: -4px;
 }
 
+.navigation-bar .item:focus {
+    outline: auto -webkit-focus-ring-color;
+}
+
 .navigation-bar .item.force-hidden {
     display: none;
 }

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/NavigationBar.js (257410 => 257411)


--- trunk/Source/WebInspectorUI/UserInterface/Views/NavigationBar.js	2020-02-26 06:05:13 UTC (rev 257410)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/NavigationBar.js	2020-02-26 07:18:43 UTC (rev 257411)
@@ -36,11 +36,10 @@
         if (label)
             this.element.setAttribute("aria-label", label);
 
-        this.element.addEventListener("focus", this._focus.bind(this), false);
-        this.element.addEventListener("blur", this._blur.bind(this), false);
         this.element.addEventListener("keydown", this._keyDown.bind(this), false);
-        this.element.addEventListener("mousedown", this._mouseDown.bind(this), false);
+        this.element.addEventListener("mousedown", this._mouseDown.bind(this), true);
 
+        this._role = role;
         this._mouseMovedEventListener = this._mouseMoved.bind(this);
         this._mouseUpEventListener = this._mouseUp.bind(this);
 
@@ -288,15 +287,18 @@
         if (event.button !== 0)
             return;
 
-        // Remove the tabIndex so clicking the navigation bar does not give it focus.
-        // Only keep the tabIndex if already focused from keyboard navigation. This matches Xcode.
-        if (!this._focused)
-            this.element.removeAttribute("tabindex");
-
         var itemElement = event.target.closest("." + WI.RadioButtonNavigationItem.StyleClassName);
         if (!itemElement || !itemElement.navigationItem)
             return;
 
+        if (this._role === "tablist") {
+            if (this.element.contains(document.activeElement)) {
+                // If clicking on a tab, stop the event from being handled by the button element. Instead,
+                // pass focus to the selected tab. Otherwise, let the button become activated normally.
+                event.stopPropagation();
+            }
+        }
+
         this._previousSelectedNavigationItem = this.selectedNavigationItem;
         this.selectedNavigationItem = itemElement.navigationItem;
 
@@ -310,8 +312,6 @@
         // Register these listeners on the document so we can track the mouse if it leaves the navigation bar.
         document.addEventListener("mousemove", this._mouseMovedEventListener, false);
         document.addEventListener("mouseup", this._mouseUpEventListener, false);
-
-        event.stopPropagation();
     }
 
     _mouseMoved(event)
@@ -373,9 +373,6 @@
 
     _keyDown(event)
     {
-        if (!this._focused)
-            return;
-
         if (event.keyIdentifier !== "Left" && event.keyIdentifier !== "Right")
             return;
 
@@ -401,18 +398,9 @@
             return;
 
         this.selectedNavigationItem = this._navigationItems[selectedNavigationItemIndex];
+        this.selectedNavigationItem?.element.focus();
     }
 
-    _focus(event)
-    {
-        this._focused = true;
-    }
-
-    _blur(event)
-    {
-        this._focused = false;
-    }
-
     _calculateMinimumWidth()
     {
         let visibleNavigationItems = this._visibleNavigationItems;

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/NavigationItem.js (257410 => 257411)


--- trunk/Source/WebInspectorUI/UserInterface/Views/NavigationItem.js	2020-02-26 06:05:13 UTC (rev 257410)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/NavigationItem.js	2020-02-26 07:18:43 UTC (rev 257411)
@@ -37,12 +37,11 @@
         this._visibilityPriority = WI.NavigationItem.VisibilityPriority.Normal;
         this._cachedWidth = NaN;
 
-        if (role)
-            this._element.setAttribute("role", role);
         if (label)
             this._element.setAttribute("aria-label", label);
 
         this._element.classList.add(...this._classNames);
+        this._element.role = role;
         this._element.navigationItem = this;
     }
 

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/RadioButtonNavigationItem.css (257410 => 257411)


--- trunk/Source/WebInspectorUI/UserInterface/Views/RadioButtonNavigationItem.css	2020-02-26 06:05:13 UTC (rev 257410)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/RadioButtonNavigationItem.css	2020-02-26 07:18:43 UTC (rev 257411)
@@ -31,6 +31,10 @@
     width: 24px;
 }
 
+.navigation-bar .item.radio.button:focus {
+    outline-offset: -1px;
+}
+
 .navigation-bar .item.radio.button.text-only {
     position: relative;
     padding: 2px 6px 4px;

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/Toolbar.css (257410 => 257411)


--- trunk/Source/WebInspectorUI/UserInterface/Views/Toolbar.css	2020-02-26 06:05:13 UTC (rev 257410)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/Toolbar.css	2020-02-26 07:18:43 UTC (rev 257411)
@@ -95,7 +95,6 @@
 
 .toolbar .item {
     display: flex;
-    outline: none;
 }
 
 .toolbar .search-bar {
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to