fix(notebook): align actions and hotkeys #5353
Conversation
There was a problem hiding this comment.
There are three states of tab visibility
- Hide all tabs
- Show only tabs that have someone streaming in them
- Show all tabs (default)
What does it mean to toggle the tab visibility? imo it means you toggle between 1 and 3
What does it mean to toggle the tab live only state? imo it means you toggle between whatever state it was in before (1 or 3) and 2
Part of the confusion comes from having different menu actions to hotkeys, but it doesn't help that the hotkeys are confusing to begin with (2 toggle actions between 3 states??)
In addition to some of the changes you've done, I propose the tab visibility is moved to a submenu with 3 always visible options matching the three states of tab visibility.
- Hide all tabs (Linked to the hotkey Set tab visibility -> Set to off)
- Show live tabs only (Linked to the hotkey Set tab visibility -> Live only on)
- Show all tabs (Linked to the hotkey Set tab visibility -> Set to on)
The downside I see to the above-mentioned change is we no longer surface the default toggle hotkeys. This could be alleviated by adding 2 more options for the toggling
src/widgets/Window.cpp
Outdated
| this->notebook_->setShowTabs(true); | ||
| getSettings()->tabVisibility.setValue( | ||
| NotebookTabVisibility::LiveOnly); | ||
| this->notebook_->toggleOfflineTabsAction()->trigger(); |
There was a problem hiding this comment.
This change feels wrong - in the hotkey selector, the value for this is "Live only on".
Right below this there's a "toggleLiveOnly" which calls this->notebook_->toggleOfflineTabs()
That's what's happening: graph TD;
All<-->|toggleLive|LiveOnly
NoneAll<-->|toggleAll|All
NoneAll-->|toggleLive|LiveOnly
NoneLive-->|toggleAll|LiveOnly
NoneLive<-->|toggleLive|All
I'm unsure about this.
Should be "Tab Visibility" → {All, Live Only, None}.
This also breaks the workflow of right-clicking and clicking on the desired option, as you now have to move through the submenu.
|
Aligns Ctrl+U and "Toggle visibility of tabs" to do the same. The hotkey will now trigger the same
QAction. I've done the same for Ctrl+Shift+L and removed the visibility filter reset when the hotkey is activated withonoroff.Fixes #5341.