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

Add ability to customize keyboard shortcuts #30840

Closed
rebron opened this issue Jun 6, 2023 · 10 comments
Closed

Add ability to customize keyboard shortcuts #30840

rebron opened this issue Jun 6, 2023 · 10 comments

Comments

@rebron
Copy link
Collaborator

rebron commented Jun 6, 2023

Description

Provide a way for users to see via a list and to find via a search field, all the available keyboard shortcuts. Users should be able to easily add/remove, edit existing keyboard shortcuts and create new ones.

Keyboard shortcuts should be made available here: brave://settings/system/ and brave://settings/system/shortcuts

Expected result:

https://www.figma.com/file/tLXWGCpNoiJxDZDdpfordj/Desktop-Settings?node-id=3617-77370&t=HKTZrtO87yiUZkol-0

Brave version (brave://version info)

Targeting 1.56.x

Version/Channel Information:

  • Can you reproduce this issue with the current release? n/a
  • Can you reproduce this issue with the beta channel? yes
  • Can you reproduce this issue with the nightly channel? yes

Other Additional Information:

  • Does the issue resolve itself when disabling Brave Shields? n/a
  • Does the issue resolve itself when disabling Brave Rewards? n/a
  • Is the issue reproducible on the latest version of Chrome? n/a

Miscellaneous Information:

Related issues:

1.56.x uplift PR:
brave/brave-core#18780

To make life easier for @kjozwiak we're going to uplift these to 1.56.x in as few PRs as possible (seeing as non of these are critical unless most of them land).

Test Plan

  1. Existing shortcuts should still work in a new profile - verify (CTRL+T for new tab, CTRL+N for new window, CTRL+L to focus the location bar.
  2. It should be possible to add new shortcuts to these commands on brave://settings/system/shortcuts
    1. Add Backspace as a shortcut for Back
    2. Press Backspace - the browser should navigate back
    3. Reset the shortcut. Backspace should no longer navigate back
    4. Add ALT+LeftArrow as a shortcut for Forward. A warning should pop up saying This combination is being used for "Back". Saving will override that shortcut.
    5. Pressing Alt+Left should now navigate forward.
    6. Add Alt+Left as a shortcut to the Back command (neither back nor forward should be resettable now, as they are already at their defaults)
  3. It should be possible to remove shortcuts from the commands
    1. Remove Alt+Left from Back. Pressing the shortcut should no longer navigate back.
    2. Add Alt+Left to Forward. No warning should appear.
  4. It should be possible to add shortcuts to commands that didn't have them previously.
    1. Add Alt+P to Pin Tab & Alt+D to duplicate tab
    2. Alt+P should toggle whether a tab is pinned. Alt+D should duplicate the current tab (same as right-click duplicate)
  5. It should be possible to reset all commands
    1. Click the Reset all to defaults button at the bottom of the page.
    2. All edited shortcuts should be reset.

Note: On macOS some shortcuts cannot be changed via the shortcuts UI. For these shortcuts, no remove button should show up, and it should not be possible to override them by adding their key combination to another shortcut.

@fallaciousreasoning
Copy link

Note for QA: It's a known issue that pressing Shift+= will display as Shift++ in the recorder (but will still be recorded properly). See #30879

@kjozwiak
Copy link
Member

kjozwiak commented Jul 6, 2023

@fallaciousreasoning @rebron should this issue be closed? I believe at this point we basically have everything merged into 1.56.x. Or should this stay in progress till #30879 is done? We have about ~week and a half left before we push out 1.56.x on July 18th.

Should #354 also be closed at this point?

CCing @LaurenWags

@fallaciousreasoning
Copy link

I think we can close this and #354, but let's have @rebron confirm. Thanks for all the uplifts/testing @kjozwiak & @LaurenWags!

@LaurenWags
Copy link
Member

LaurenWags commented Jul 7, 2023

if we close this one out for 1.56.x we should have a test plan for it - we've verified various issues as listed above but afaik we still don't have a feature test plan, only the individual issues. I believe @rebron mentioned he'd work with you @fallaciousreasoning to get a test plan together?

@fallaciousreasoning
Copy link

Added a test plan to this issue 😄 Let me know if you think there's anything missing

@LaurenWags
Copy link
Member

@fallaciousreasoning is there anything Windows (or Linux) specific we should be aware of when testing this one?

cc @brave/qa-team

@stephendonner
Copy link

stephendonner commented Jul 10, 2023

Verification PASSED using

Brave | 1.56.1 Chromium: 115.0.5790.75 (Official Build) (x86_64)
-- | --
Revision | 3433e4d513e8454ba0243deea3d53693661dc0c3-refs/branch-heads/5790@{#1430}
OS | macOS Version 11.7.8 (Build 20G1351)

Shared Steps:

  1. installed 1.56.1
  2. launched Brave
  3. opened brave://flags
  4. set brave://flags/#brave-commands to Enabled
  5. clicked Relaunch
  6. opened brave://settings/
  7. clicked on System
  8. clicked on Shortcuts
brave://flags brave://settings/system/shortcuts
Screen Shot 2023-07-10 at 1 52 59 PM Screen Shot 2023-07-10 at 1 53 15 PM

Ensure (pre)existing shortcuts continue to work - PASSED

Ensured all of the following commands' keybindings/shortcuts worked:

  • New tab - command + T
  • New window - command + N
  • Focus location bar - command + L
  • Bookmark this tab - command + D
  • Back - command + {
  • Forward - command + }

Add new shortcuts to existing commands - PASSED

(continued from Shared Steps)

Back binding - PASSED

Steps:

  1. clicked on Add next to Back
  2. pressed delete (Backspace)
  3. clicked Save
  4. confirmed Back showed two keybindings/shortcuts
  5. confirmed both worked
  6. scrolled all the way down and clicked on Reset all to defaults
  7. confirmed the added shortcut was removed
  8. confirmed command + [ still navigated backwords
example example example example example example
Screen Shot 2023-07-10 at 3 06 38 PM Screen Shot 2023-07-10 at 3 06 45 PM Screen Shot 2023-07-10 at 3 06 49 PM Screen Shot 2023-07-10 at 3 11 56 PM Screen Shot 2023-07-10 at 3 12 04 PM Screen Shot 2023-07-10 at 3 06 49 PM

(continued from Shared Steps

Forward binding - PASSED

Steps:

  1. clicked on Add next to Forward
  2. pressed command + left arrow
  3. clicked Save
  4. confirmed Forward showed two keybindings/shortcuts
  5. confirmed they both navigated forwards
example example example
Screen Shot 2023-07-10 at 3 16 43 PM Screen Shot 2023-07-10 at 3 29 36 PM Screen Shot 2023-07-10 at 3 16 49 PM

Remove added shortcuts - PASSED

(continued from `Shared Steps`)

Steps:

  1. added command + left arrow to Back
  2. added command + right arrow to Forward
  3. confirmed both worked
  4. confirmed both had Reset next to Add, now
  5. hovered over both command + left arrow & command+right arrow`
  6. confirmed both had -
  7. clicked on them
  8. confirmed Reset disappears for both, as they are now back to their defaults
example example example example
Screen Shot 2023-07-10 at 3 36 42 PM Screen Shot 2023-07-10 at 3 41 01 PM Screen Shot 2023-07-10 at 3 41 08 PM Screen Shot 2023-07-10 at 3 41 11 PM

Add new shortcuts as a new command - PASSED

Steps:

(continued from Shared Steps)

  1. searched for pin tab
  2. clicked Add
  3. pressed control + P
  4. clicked Save
  5. pressed control + P
  6. confirmed the current tab became pinned
example example example example
Screen Shot 2023-07-10 at 2 34 11 PM Screen Shot 2023-07-10 at 2 34 47 PM Screen Shot 2023-07-10 at 2 34 59 PM Screen Shot 2023-07-10 at 2 35 10 PM

Unable to change system-level commands/key bindings - PASSED

(continued from Shared Steps)

Steps:

  1. searched for back
  2. confirmed no ability to remove the keybinding shortcut of command + [
  3. clicked on Add for the next binding
  4. tried to bind using command + [
  5. confirmed I got an error about it being a system shortcut, and where to modify it, in System Settings
  6. also confirmed with Print, Copy, etc.
example example example example
Screen Shot 2023-07-10 at 2 47 23 PM Screen Shot 2023-07-10 at 2 50 32 PM Screen Shot 2023-07-12 at 1 36 50 PM Screen Shot 2023-07-12 at 1 37 38 PM

Adding duplicate shortcuts - FAILED

(continued from Shared Steps)

  1. clicked on Add next to Web app info
  2. pressed Command + D
  3. clicked Save
  4. pressed Command + D

Expected:

Should open Web app info / not be allowed to be assigned as a shortcut

Actual:

Able to visibly create the assigned shortcut for Web app info, but it still works as regular Bookmark Tab

example example example
Screen Shot 2023-07-11 at 3 35 43 AM Screen Shot 2023-07-11 at 3 35 48 AM Screen Shot 2023-07-11 at 3 42 14 AM

NOTE: Retest with latest nightly build after #31552 lands

Tested using

Brave | 1.56.3 Chromium: 115.0.5790.75 (Official Build) (x86_64)
-- | --
Revision | 3433e4d513e8454ba0243deea3d53693661dc0c3-refs/branch-heads/5790@{#1430}
OS | macOS Version 11.7.8 (Build 20G1351)

Reset all commands - PASSED

(continued from Shared Steps)

Steps:

  1. assigned non-conflicting shortcuts to previously unassigned commands (Password Manager - Show app menu)
  2. confirmed they worked
  3. scrolled down to the bottom
  4. clicked on Reset all to defaults
  5. scrolled back up
  6. confirmed shortcuts for Password Manager - Show app menu were reset and unassigned
  7. confirmed the previous shortcuts no longer worked
example example example
Screen Shot 2023-07-12 at 1 29 59 PM Screen Shot 2023-07-12 at 1 46 03 PM Screen Shot 2023-07-12 at 1 46 33 PM

Light mode - PASSED

example example example
Screen Shot 2023-07-10 at 4 33 32 PM Screen Shot 2023-07-10 at 4 33 39 PM Screen Shot 2023-07-10 at 4 33 54 PM

Logged/encountered:

@stephendonner stephendonner added the QA/In-Progress Indicates that QA is currently in progress for that particular issue label Jul 10, 2023
@fallaciousreasoning
Copy link

@fallaciousreasoning is there anything Windows (or Linux) specific we should be aware of when testing this one?

Nope, there's just the MacOS thing, where some shortcuts can't be edited.

@MadhaviSeelam
Copy link

MadhaviSeelam commented Jul 11, 2023

Verification PASSED using

Brave | 1.56.1 Chromium: 115.0.5790.75 (Official Build) (64-bit)
-- | --
Revision | 3433e4d513e8454ba0243deea3d53693661dc0c3-refs/branch-heads/5790@{#1430}
OS | Windows 11 Version 22H2 (Build 22621.1928)

Shared Steps:

  1. installed 1.56.1
  2. launched Brave
  3. opened brave://flags
  4. set brave://flags/#brave-commands to Enabled
  5. clicked Relaunch
  6. opened brave://settings/
  7. clicked on System
  8. clicked on Shortcuts
brave://flags brave://settings/system/shortcuts
image image

Ensure (pre)existing shortcuts continue to work - PASSED

Ensured all of the following commands' keybindings/shortcuts worked:

  • New tab - CTRL + T
  • New window - CTRL+N + N
  • Focus location bar - CTRL+L
  • Bookmark this tab - CTRL + D
  • Back - Alt + ArrowLeft
  • Forward - Alt + ArrowRight

Add new shortcuts to existing commands - PASSED

(continued from Shared Steps)

Back shortcut- PASSED

Steps:

  1. clicked on Add in the Back section
  2. pressed Backspace as a new shortcut
  3. clicked Save
  4. confirmed Backspace shortcut is added
  5. confirmed Alt+BrrowLeft& Backspace worked
  6. clicked Reset.
  7. confirmed Backspace no longer navigated back
example example example example example example
image image image image image image

Forward shortcut

clicked on Add in the Forward section
Press ALT+LeftArrow as a shortcut
A warning message shown This combination is being used for "Back". Saving will override that shortcut.
clicked Save
verified Alt+ArrowLeft is switched from Back to Forward
confirmed pressing Alt+ArrowLeft navigated forward.
clicked Add to add back Alt+ArrowLeft Or Reset
confirmed Alt+ArrowLeft is added to BackandResetoption is not available forBack&Forward` sections

example example example
image image image

Remove newly added and existing shortcuts - PASSED

(continued from Shared Steps)

Steps:

  1. Removed existing Alt + ArrowLeft from Back
  2. confirmed the shortcut no longer navigated back
  3. Added Backspace to Back and verified it worked
  4. removed Backspace
  5. confirmed Backspace was removed and the shortcut no longer navigated back
  6. added Alt + ArrowLeft to Forward
  7. confirmed no warning message shown when Alt + ArrowLeft and verified that it navigated forward
  8. hovered over both Alt + ArrowLeft
  9. removed the shortcut via '-` from Forward section
  10. confirmed Reset disappears for both, as they are now back to their defaults
  11. clicked Reset in the `Back section
  12. Alt + ArrowLeft shortcut shown in the Back section
example example example example example example example
image image image image image image image

Add new shortcuts as a new command - PASSED

Steps:

(continued from Shared Steps)

  1. searched for pin
  2. clicked Add
  3. pressed Alt + P
  4. clicked Save
  5. pressed Alt + P
  6. confirmed the current tab became pinned
  7. confirmed able to toggle the pinned /unpinned tab with Alt+P
  8. searched for Duplicate
  9. added and over rided Alt+D shortcut
  10. confirmed able to duplicate the tab with Alt+D shortcut
example example example example
image image image image

Reset all commands - PASSED

  1. Added couple shortcuts for Pin tab (Alt+P) and Pin this page to Start screen.. (Cntrl + Alt + P)
  2. click Reset all to defaults at the bottom of the page
  3. both entries should be cleared
  4. Edit existing shortcut for Focus bo0kmarks (Alt + Shift + B) to

image

Dark mode - PASSED

![image](https://github.com/brave/brave-browser/assets/98358127/c710da0f-c3f6-41a7-9424-2a3bce9c6275)

Upgrade - PASSED

  1. Install 1.52.130
  2. launch Brave
  3. enabled #brave-commands in brave://flags
  4. close the browser
  5. shortcuts entry is not available in brave://settings/system as expected
  6. launched Brave with 1.56.1
  7. clicked shortcuts in `brave://settings/system

Confirmed Shortcuts UI is shown in brave://settings/system/shortcuts
Shortcuts are working as expected

example example example example
image image image image

@GeetaSarvadnya
Copy link

GeetaSarvadnya commented Jul 13, 2023

Verification PASSED on

Brave	1.56.5 Chromium: 115.0.5790.75 (Official Build) (64-bit) 
Revision	3433e4d513e8454ba0243deea3d53693661dc0c3-refs/branch-heads/5790@{#1430}
OS	Linux

Shared Steps:

  1. installed 1.56.1
  2. launched Brave
  3. opened brave://flags
  4. set brave://flags/#brave-commands to Enabled
  5. clicked Relaunch
  6. opened brave://settings/
  7. clicked on System
  8. clicked on Shortcuts
brave://flags brave://settings/system/shortcuts 1 2 3 4 5 6 7 8 9 10 11 12 13 14
image image image image image image image image image image image image image image image image

Ensure (pre)existing shortcuts continue to work - PASSED

Ensured all of the following commands' keybindings/shortcuts worked:

  • New tab - CTRL + T
  • New window - CTRL + N
  • Focus location bar - CTRL + L
  • Bookmark this tab - CTRL + D
  • Back - ALT + ArrowLeft
  • Forward - ALT + ArrowRight

Added Backspace as shortcut for Back cmd ensured when user press Backspace - the browser navigates back
image

Add new shortcuts to existing commands - PASSED

(continued from Shared Steps)

Back binding - PASSED

Steps:

  1. clicked on Add next to Back
  2. pressed delete (Backspace)
  3. clicked Save
  4. confirmed Back showed two keybindings/shortcuts
  5. confirmed both worked
  6. scrolled all the way down and clicked on Reset all to defaults
  7. confirmed the added shortcut was removed
  8. confirmed ALT + Arrowleft still navigated backwords
example example example example example example
image image image image image image

(continued from Shared Steps

Forward binding - PASSED

Steps:

  1. clicked on Add next to Forward
  2. pressed ALT + left arrow
  3. clicked Save
  4. confirmed Forward showed two keybindings/shortcuts
  5. confirmed they both navigated forwards
example example
image image

Remove added shortcuts - PASSED

(continued from `Shared Steps`)

Steps:

  1. added ALT + ArrowRight to Back
  2. added ALT + ArrowLeft to Forward
  3. confirmed both worked
  4. confirmed both had Reset next to Add, now
  5. hovered over both ALT + ArrowRight & ALT + ArrowLeft
  6. confirmed both had -
  7. clicked on them
  8. confirmed Reset disappears for both, as they are now back to their defaults
example example example example example
image image image image image

Add new shortcuts as a new command - PASSED

Steps:

(continued from Shared Steps)

  1. searched for pin tab
  2. clicked Add
  3. pressed control + P
  4. clicked Save
  5. pressed control + P
  6. confirmed the current tab became pinned
example example example example example
image image image image image

Reset all commands - PASSED

  1. Added couple shortcuts for Pin tab (Alt+P) and Pin this page to Start screen.. (Cntrl + Alt + P)
  2. click Reset all to defaults at the bottom of the page
  3. both entries should be cleared
Example Example
image image

@rebron rebron added this to the 1.56.x - Release milestone Jul 18, 2023
@rebron rebron changed the title Add keyboard shortcuts Add ability to customize keyboard shortcuts Jul 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment