brave / brave-browser

Brave browser for Android, iOS, Linux, macOS, Windows.
https://brave.com
Mozilla Public License 2.0
17.63k stars 2.29k forks source link

"Toggle Vertical Tabs Expanded" keyboard shortcut not working when "Expand Vertical Tabs Independently per Window" is enabled #41074

Open richardso21 opened 2 weeks ago

richardso21 commented 2 weeks ago

Description

This is an extension to #36224, which introduced the ability for each window to have an independent state whether its vertical tab is expanded. However, this causes the custom keyboard shortcut "Toggle Vertical Tabs Expanded" to have no visible effect at all when this per-window state is enabled.

Instead, this shortcut toggles the global expanded state across all windows (which is the default behavior for the expansion/collapse of vertical tabs for Brave). I know this because if I enable per-window expansion, invoke this keyboard shortcut once, then disable per-window expansion, the vertical tab menu is left at a state opposite from before performing this sequence of steps (so if the tabs were expanded before, then it would be collapsed after, and vice versa). Invoking this shortcut twice, however, results in the state being the same.

Steps to reproduce

  1. Enable vertical tabs (Appearance -> Use Vertical Tabs)
  2. Enable independent vertical tabs expand state per window (check "Expand vertical tabs independently per window")
  3. Assign a keybind to the "Toggle Vertical Tabs Expanded" option
  4. Try invoking the keybind

Actual result

Nothing happens visibly, however, you can observe the state of the global vertical tab has changed once you disable independent vertical tab expand state.

Expected result

The state of the window-independent vertical tab menu toggle from expanded to collapsed, or vice versa.

Reproduces how often

Easily reproduced

Brave version (brave://version info)

Beta Brave 1.70.107 Chromium: 128.0.6613.120 (Official Build) beta (arm64)
Revision ab3f504ca4a15c330f60a93d5e3773d780498980
OS macOS Version 15.0 (Build 24A335)
Nightly Brave 1.72.6 Chromium: 129.0.6668.42 (Official Build) nightly (arm64)
Revision 2071656859878e424523b78457dc7a21d7f8d01c
OS macOS Version 15.0 (Build 24A335)

Channel information

Reproducibility

Miscellaneous information

I attempted to fix this on my own, however, I did not have enough disk storage to npm run init the brave-core repository on my machine 😅. However, I believe that this function is close to the root of this problem: https://github.com/brave/brave-core/blob/e9b85bbcaf46c9111f0712cd920f53516107b8fe/browser/ui/browser_commands.cc#L326. We can have a separate block of logic that's run if independent expanded tab state is enabled. An alternative would be to refactor brave_tabs::kVerticalTabsCollapsed to respect the window-independent states when the option is enabled.

richardso21 commented 1 week ago

Now that release has caught up to 1.70, it now has the new "Expand Vertical Tabs Independently per Window" feature, and it also faces the same issue as beta as nightly channels.