Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1829393002:
Add keyboard accessibility to ConstrainedWindowButton (Closed)
|
Created:
4 years, 9 months ago by juncai Modified:
4 years, 8 months ago Reviewers:
Robert Sesek CC:
chromium-reviews, Reilly Grant (use Gerrit) Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd keyboard accessibility to ConstrainedWindowButton
This patch added keyboard accessibility to ConstrainedWindowButton.
ConstrainedWindowButton is used by WebUSB chooser bubble,
Geolocation permission bubble, etc. Previously, when trying to only
interact with these bubbles with the keyboard, there are no focus
styles. User should be able to press tab to navigate to buttons, and
see a visible focus state. It will allow the user to only use keyboard
to control the bubble, which improves the accessibility of the UI
that uses it.
BUG=596671
Committed: https://crrev.com/9f0ead0fd04825415795733a60d1cf587fd12b2a
Cr-Commit-Position: refs/heads/master@{#383368}
Patch Set 1 : added keyboard accessibility to ConstrainedWindowButton #
Total comments: 2
Patch Set 2 : address rsesek@'s comment #
Messages
Total messages: 21 (10 generated)
Description was changed from ========== Add keyboard accessibility to ConstrainedWindowButton This patch added keyboard accessibility to ConstrainedWindowButton, which is used by WebUSB chooser bubble, Geolocation permission bubble, etc. It will allow the user to only use keyboard to control the bubble, which improves the accessibility of the UI that uses it. BUG=596671 ========== to ========== Add keyboard accessibility to ConstrainedWindowButton This patch added keyboard accessibility to ConstrainedWindowButton, and added code to handle the event when "Enter" is pressed. ConstrainedWindowButton is used by WebUSB chooser bubble, Geolocation permission bubble, etc. Previously, when trying to only interact with these bubbles with the keyboard, there are no focus styles. User should be able to press tab to navigate to buttons, and see a visible focus state. It will allow the user to only use keyboard to control the bubble, which improves the accessibility of the UI that uses it. BUG=596671 ==========
Description was changed from ========== Add keyboard accessibility to ConstrainedWindowButton This patch added keyboard accessibility to ConstrainedWindowButton, and added code to handle the event when "Enter" is pressed. ConstrainedWindowButton is used by WebUSB chooser bubble, Geolocation permission bubble, etc. Previously, when trying to only interact with these bubbles with the keyboard, there are no focus styles. User should be able to press tab to navigate to buttons, and see a visible focus state. It will allow the user to only use keyboard to control the bubble, which improves the accessibility of the UI that uses it. BUG=596671 ========== to ========== Add keyboard accessibility to ConstrainedWindowButton This patch added keyboard accessibility to ConstrainedWindowButton, and added code to handle the event when "Enter" is pressed. ConstrainedWindowButton is used by WebUSB chooser bubble, Geolocation permission bubble, etc. Previously, when trying to only interact with these bubbles with the keyboard, there are no focus styles. User should be able to press tab to navigate to buttons, and see a visible focus state. It will allow the user to only use keyboard to control the bubble, which improves the accessibility of the UI that uses it. BUG=596671 ==========
Description was changed from ========== Add keyboard accessibility to ConstrainedWindowButton This patch added keyboard accessibility to ConstrainedWindowButton, and added code to handle the event when "Enter" is pressed. ConstrainedWindowButton is used by WebUSB chooser bubble, Geolocation permission bubble, etc. Previously, when trying to only interact with these bubbles with the keyboard, there are no focus styles. User should be able to press tab to navigate to buttons, and see a visible focus state. It will allow the user to only use keyboard to control the bubble, which improves the accessibility of the UI that uses it. BUG=596671 ========== to ========== Add keyboard accessibility to ConstrainedWindowButton This patch added keyboard accessibility to ConstrainedWindowButton, and added code to handle the event when "Enter" is pressed. ConstrainedWindowButton is used by WebUSB chooser bubble, Geolocation permission bubble, etc. Previously, when trying to only interact with these bubbles with the keyboard, there are no focus styles. User should be able to press tab to navigate to buttons, and see a visible focus state. It will allow the user to only use keyboard to control the bubble, which improves the accessibility of the UI that uses it. BUG=596671 ==========
juncai@chromium.org changed reviewers: + rsesek@chromium.org
Please take a look.
Description was changed from ========== Add keyboard accessibility to ConstrainedWindowButton This patch added keyboard accessibility to ConstrainedWindowButton, and added code to handle the event when "Enter" is pressed. ConstrainedWindowButton is used by WebUSB chooser bubble, Geolocation permission bubble, etc. Previously, when trying to only interact with these bubbles with the keyboard, there are no focus styles. User should be able to press tab to navigate to buttons, and see a visible focus state. It will allow the user to only use keyboard to control the bubble, which improves the accessibility of the UI that uses it. BUG=596671 ========== to ========== Add keyboard accessibility to ConstrainedWindowButton This patch added keyboard accessibility to ConstrainedWindowButton, and added code to handle the keyboard event when "Enter" is pressed. ConstrainedWindowButton is used by WebUSB chooser bubble, Geolocation permission bubble, etc. Previously, when trying to only interact with these bubbles with the keyboard, there are no focus styles. User should be able to press tab to navigate to buttons, and see a visible focus state. It will allow the user to only use keyboard to control the bubble, which improves the accessibility of the UI that uses it. BUG=596671 ==========
https://codereview.chromium.org/1829393002/diff/1/chrome/browser/ui/cocoa/con... File chrome/browser/ui/cocoa/constrained_window/constrained_window_button.mm (right): https://codereview.chromium.org/1829393002/diff/1/chrome/browser/ui/cocoa/con... chrome/browser/ui/cocoa/constrained_window/constrained_window_button.mm:258: if ([event keyCode] == kVK_Return) Return is not typically how buttons are invoked on OS X, it's usually space. Does that work without this change to -keyDown:?
Description was changed from ========== Add keyboard accessibility to ConstrainedWindowButton This patch added keyboard accessibility to ConstrainedWindowButton, and added code to handle the keyboard event when "Enter" is pressed. ConstrainedWindowButton is used by WebUSB chooser bubble, Geolocation permission bubble, etc. Previously, when trying to only interact with these bubbles with the keyboard, there are no focus styles. User should be able to press tab to navigate to buttons, and see a visible focus state. It will allow the user to only use keyboard to control the bubble, which improves the accessibility of the UI that uses it. BUG=596671 ========== to ========== Add keyboard accessibility to ConstrainedWindowButton This patch added keyboard accessibility to ConstrainedWindowButton. ConstrainedWindowButton is used by WebUSB chooser bubble, Geolocation permission bubble, etc. Previously, when trying to only interact with these bubbles with the keyboard, there are no focus styles. User should be able to press tab to navigate to buttons, and see a visible focus state. It will allow the user to only use keyboard to control the bubble, which improves the accessibility of the UI that uses it. BUG=596671 ==========
https://codereview.chromium.org/1829393002/diff/1/chrome/browser/ui/cocoa/con... File chrome/browser/ui/cocoa/constrained_window/constrained_window_button.mm (right): https://codereview.chromium.org/1829393002/diff/1/chrome/browser/ui/cocoa/con... chrome/browser/ui/cocoa/constrained_window/constrained_window_button.mm:258: if ([event keyCode] == kVK_Return) On 2016/03/25 20:16:03, Robert Sesek wrote: > Return is not typically how buttons are invoked on OS X, it's usually space. > Does that work without this change to -keyDown:? The space works without this change to -keyDown:. So this method is removed. Done.
LGTM
The CQ bit was checked by juncai@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1829393002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1829393002/20001
Message was sent while issue was closed.
Description was changed from ========== Add keyboard accessibility to ConstrainedWindowButton This patch added keyboard accessibility to ConstrainedWindowButton. ConstrainedWindowButton is used by WebUSB chooser bubble, Geolocation permission bubble, etc. Previously, when trying to only interact with these bubbles with the keyboard, there are no focus styles. User should be able to press tab to navigate to buttons, and see a visible focus state. It will allow the user to only use keyboard to control the bubble, which improves the accessibility of the UI that uses it. BUG=596671 ========== to ========== Add keyboard accessibility to ConstrainedWindowButton This patch added keyboard accessibility to ConstrainedWindowButton. ConstrainedWindowButton is used by WebUSB chooser bubble, Geolocation permission bubble, etc. Previously, when trying to only interact with these bubbles with the keyboard, there are no focus styles. User should be able to press tab to navigate to buttons, and see a visible focus state. It will allow the user to only use keyboard to control the bubble, which improves the accessibility of the UI that uses it. BUG=596671 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Add keyboard accessibility to ConstrainedWindowButton This patch added keyboard accessibility to ConstrainedWindowButton. ConstrainedWindowButton is used by WebUSB chooser bubble, Geolocation permission bubble, etc. Previously, when trying to only interact with these bubbles with the keyboard, there are no focus styles. User should be able to press tab to navigate to buttons, and see a visible focus state. It will allow the user to only use keyboard to control the bubble, which improves the accessibility of the UI that uses it. BUG=596671 ========== to ========== Add keyboard accessibility to ConstrainedWindowButton This patch added keyboard accessibility to ConstrainedWindowButton. ConstrainedWindowButton is used by WebUSB chooser bubble, Geolocation permission bubble, etc. Previously, when trying to only interact with these bubbles with the keyboard, there are no focus styles. User should be able to press tab to navigate to buttons, and see a visible focus state. It will allow the user to only use keyboard to control the bubble, which improves the accessibility of the UI that uses it. BUG=596671 Committed: https://crrev.com/9f0ead0fd04825415795733a60d1cf587fd12b2a Cr-Commit-Position: refs/heads/master@{#383368} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/9f0ead0fd04825415795733a60d1cf587fd12b2a Cr-Commit-Position: refs/heads/master@{#383368}
Message was sent while issue was closed.
ah, I found that there is a setting on Mac that can control the keyboard access: System Preferences -> Keyboard -> Shortcuts At the bottom, it has: "Full Keyboard Access: In windows and dialogs, press Tab to move keyboard focus between: Text boxes and lists only All controls" It seems that the first "Text boxes and lists only" is default, that's why when I tested the bubble, pressing the tab can't navigate the buttons. With the change in this patch, pressing tab works. When I changed the setting to "All controls", without the change in this patch, pressing the tab can navigate the buttons and space works like pressing the button. I am wondering if the change in this patch is needed. It does have the benefit that under the default setting, user still can use tab to navigate the button and control it.
Message was sent while issue was closed.
Description was changed from ========== Add keyboard accessibility to ConstrainedWindowButton This patch added keyboard accessibility to ConstrainedWindowButton. ConstrainedWindowButton is used by WebUSB chooser bubble, Geolocation permission bubble, etc. Previously, when trying to only interact with these bubbles with the keyboard, there are no focus styles. User should be able to press tab to navigate to buttons, and see a visible focus state. It will allow the user to only use keyboard to control the bubble, which improves the accessibility of the UI that uses it. BUG=596671 Committed: https://crrev.com/9f0ead0fd04825415795733a60d1cf587fd12b2a Cr-Commit-Position: refs/heads/master@{#383368} ========== to ========== Add keyboard accessibility to ConstrainedWindowButton This patch added keyboard accessibility to ConstrainedWindowButton. ConstrainedWindowButton is used by WebUSB chooser bubble, Geolocation permission bubble, etc. Previously, when trying to only interact with these bubbles with the keyboard, there are no focus styles. User should be able to press tab to navigate to buttons, and see a visible focus state. It will allow the user to only use keyboard to control the bubble, which improves the accessibility of the UI that uses it. BUG=596671 Committed: https://crrev.com/9f0ead0fd04825415795733a60d1cf587fd12b2a Cr-Commit-Position: refs/heads/master@{#383368} ==========
Message was sent while issue was closed.
On 2016/03/25 22:33:36, juncai wrote: > ah, I found that there is a setting on Mac that can control the keyboard access: > > System Preferences -> Keyboard -> Shortcuts > > At the bottom, it has: > "Full Keyboard Access: In windows and dialogs, press Tab to move keyboard focus > between: > Text boxes and lists only > All controls" > > It seems that the first "Text boxes and lists only" is default, that's why when > I tested the bubble, pressing the tab can't navigate the buttons. With the > change in this patch, pressing tab works. > > When I changed the setting to "All controls", without the change in this patch, > pressing the tab can navigate the buttons and space works like pressing the > button. > > I am wondering if the change in this patch is needed. It does have the benefit > that under the default setting, user still can use tab to navigate the button > and control it. No, if it works with All Controls enabled without this patch, then we should revert this patch.
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:20001) has been created in https://codereview.chromium.org/1837803002/ by juncai@chromium.org. The reason for reverting is: There is a setting on Mac that can control the keyboard access: System Preferences -> Keyboard -> Shortcuts At the bottom, it has: "Full Keyboard Access: In windows and dialogs, press Tab to move keyboard focus between: Text boxes and lists only All controls" When setting to "All controls", without the change in this patch, pressing the tab can navigate the buttons and space works like pressing the button. So this patch is not needed.. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
