Add keyboard shortcuts to titles #5857

kommunarr · 2024-10-13T23:32:04Z

Add keyboard shortcuts to titles

Pull Request Type

Bugfix
Feature Implementation
Documentation
Other

Related issue

closes #1587

Description

Adds keyboard shortcut information to their button labels, e.g., Mute (m)
- This includes the video player buttons, the feed refresh buttons, the side nav History/Settings titles, and the top nav HistoryForward/HistoryBackward/OpenNewWindow buttons, but NOT Electron context menu labels
Implementation note: To do this properly for the built-in Shaka controls that we don't modify in our code, this PR updates the base Shaka label localization values to include the shortcut whenever the Shaka locale is set.
Implementation note: Constants were added for the aforementioned keyboard shortcuts.

Screenshots

Testing

Test that buttons with corresponding keyboard shortcuts have the shortcut referenced in the label, be they in "on" or "off" mode (e.g., Full screen (f) / Exit full screen (f)). See the official list of shortcuts for reference.

Desktop

OS: OpenSUSE
OS Version: TW

Additional context

The addition of the new constants should help make enhancements like #251 easier in the future.

…stants

src/constants.js

PikachuEXE · 2024-10-14T05:51:41Z

Label for captions

kommunarr · 2024-10-14T13:18:10Z

Another good catch; I wasn't testing with a video that had a captions option. Just fixed the issue and added logic preventing label modification if the Shaka localization key does not exist. Also note that this specific Captions label will be the only one with another parenthetical present. This is fine IMO.

…has no matching value

src/constants.js

efb4f5ff-1298-471a-8973-3d47447115dc

Pressing c doesnt enable/disable captions

kommunarr · 2024-10-14T18:59:15Z

Added localization of shortcuts & multi-key shortcuts and caught a dangling instance of the wrong constant name for CAPTIONS

efb4f5ff-1298-471a-8973-3d47447115dc · 2024-10-14T19:04:55Z

Question: Navigating to the History tab and Settings tab also got shortcuts. Is there a reason those weren't added?

…onstants

kommunarr · 2024-10-14T19:11:31Z

@efb4f5ff-1298-471a-8973-3d47447115dc I specifically added the shortcuts with corresponding button labels. The closest equivalent to those would be the corresponding side nav options, which I suppose would work. I can add those as well.

…e naming

PikachuEXE

Tested video player, sidebar, top nav button labels, refresh button

Only on MacOS though

kommunarr · 2024-10-20T20:43:44Z

Still on the fence about whether we should use the Mac key symbols for darwin (⌘/⇧/⌥/etc.)

Edit: did it. I used the version of the arrow icons that are used in Mac context menus, but it is worth noting that FF for Mac uses the more standard-looking ←→↑↓. I'm also open to using the arrow icons for non-Mac systems; I just went with the text version of those from looking at FF for Linux.

efb4f5ff-1298-471a-8973-3d47447115dc · 2024-10-23T00:23:04Z

Still on the fence about whether we should use the Mac key symbols for darwin (⌘/⇧/⌥/etc.)

Edit: did it. I used the version of the arrow icons that are used in Mac context menus, but it is worth noting that FF for Mac uses the more standard-looking ←→↑↓. I'm also open to using the arrow icons for non-Mac systems; I just went with the text version of those from looking at FF for Linux.

@PikachuEXE what do you think of the change in the latest commit

PikachuEXE · 2024-10-23T00:58:32Z

I think there is no need to use short form on labels
The places I see short forms are app menu bar & keymap menus with limited spaces
Even short forms should be used the arrows chars should be ←→↑↓ at least
I haven't checked how this looks on Windows yet

Update 1: Checked windows version

MacOS only symbol: prefer text, probably fine with symbol too if someone else want it
Arrow: Just use text

Zen Browser

RubyMine

Vivaldi

kommunarr · 2024-10-23T01:36:07Z

I can change the arrow symbols. I'm not sure if we have other active contributors who are Mac users to provide additional opinions, but maybe if any users have any thoughts

kommunarr · 2024-10-23T13:25:12Z

Updated the arrow icons for Mac:

…board shorcut modal

absidue · 2024-10-28T10:59:44Z

src/renderer/components/ft-shaka-video-player/ft-shaka-video-player.js

+      // Add the keyboard shortcut to the label for the default Shaka controls
+
+      const shakaControlKeysToShortcutLocalizations = new Map()
+      Object.entries(shakaControlKeysToShortcuts).forEach(([shakaControlKey, shortcut]) => {
+        const originalLocalization = localization.resolve(shakaControlKey)
+        if (originalLocalization === '') {
+          // e.g., A Shaka localization key in shakaControlKeysToShortcuts has fallen out of date and need to be updated
+          console.error('Mising Shaka localization key "%s"', shakaControlKey)
+          return
+        }
+
+        const localizationWithShortcut = addKeyboardShortcutToActionTitle(
+          originalLocalization,
+          shortcut
+        )
+
+        shakaControlKeysToShortcutLocalizations.set(shakaControlKey, localizationWithShortcut)
+      })
+
+      localization.insert(shakaLocale, shakaControlKeysToShortcutLocalizations)


Non-blocking: For locales that shaka-player doesn't pre-bundle (so the ones loaded by the code above), we call .insert() twice, once to add the strings and the second time to overwrite them with the shortcuts text. So in the future it might be worth considering having two separate code paths, the current one would be used for locales that are pre-bundled and a second one that combines the fetching and modifications in a single step so we only need to insert the strings once.

absidue · 2024-10-28T13:59:57Z

I preferred the stats button before this pull request a lot more as it matched the appearance of the other controls, now it looks out of place in the player.

Before:

After:

Other controls:

kommunarr · 2024-10-28T14:27:36Z

Ah, for the stats button, you're referring to the text content? I noticed that the labels we were setting for it weren't being used, so I thought that was an error. I can change that back if desired.

I can also look into removing the shortcuts from the overflow menu if that is desired as well. My only concern with that is that I'm not sure if we will be adding more shortcuts or moving more controls to a default overflow menu in the future.

efb4f5ff-1298-471a-8973-3d47447115dc

Fullwinodow doesnt show shortcut key in overflow menu

kommunarr · 2024-10-31T16:20:20Z

Sorry, I'm still a bit confused on what is being asked to be changed. Just fixing full window to have the shortcut show in the overflow menu? @absidue @efb4f5ff-1298-471a-8973-3d47447115dc

github-actions · 2024-11-02T10:37:11Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

efb4f5ff-1298-471a-8973-3d47447115dc · 2024-11-02T14:44:27Z

Just fixing full window to have the shortcut show in the overflow menu?

correct

kommunarr added 2 commits October 13, 2024 10:53

Update title for custom FreeTube button labels with keyboard shortcuts

fe5de7f

Add keyboard shortcut to unmodified Shaka control labels

3134053

github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Oct 13, 2024

FreeTubeBot enabled auto-merge (squash) October 13, 2024 23:32

Replace in-code use of available shortcuts with the corresponding con…

0d3882e

…stants

kommunarr force-pushed the feat/add-shortcut-labels branch from 24b233d to 0d3882e Compare October 13, 2024 23:35

kommunarr requested a review from efb4f5ff-1298-471a-8973-3d47447115dc October 13, 2024 23:35

PikachuEXE reviewed Oct 14, 2024

View reviewed changes

src/constants.js Outdated Show resolved Hide resolved

src/constants.js Outdated Show resolved Hide resolved

Add explanatory comments

443c4af

kommunarr requested a review from PikachuEXE October 14, 2024 02:35

Fix captions constant name

42c3128

kommunarr force-pushed the feat/add-shortcut-labels branch from ad59178 to 6202ff7 Compare October 14, 2024 13:25

Prevent creating new shortcut localization if Shaka localization key …

7c379e2

…has no matching value

kommunarr force-pushed the feat/add-shortcut-labels branch from 6202ff7 to 7c379e2 Compare October 14, 2024 13:26

ChunkyProgrammer reviewed Oct 14, 2024

View reviewed changes

src/constants.js Outdated Show resolved Hide resolved

efb4f5ff-1298-471a-8973-3d47447115dc reviewed Oct 14, 2024

View reviewed changes

Add util functions to localize special keys

764cace

kommunarr requested review from ChunkyProgrammer and efb4f5ff-1298-471a-8973-3d47447115dc October 14, 2024 18:59

Replace more video player shortcut usage in code with corresponding c…

6798c3a

…onstants

Add labels for History and Settings app shortcuts, and update variabl…

6f8d212

…e naming

kommunarr force-pushed the feat/add-shortcut-labels branch from b934703 to 6f8d212 Compare October 14, 2024 19:35

efb4f5ff-1298-471a-8973-3d47447115dc previously approved these changes Oct 14, 2024

View reviewed changes

PikachuEXE previously approved these changes Oct 15, 2024

View reviewed changes

ChunkyProgrammer requested a review from absidue October 15, 2024 00:54

PikachuEXE mentioned this pull request Oct 15, 2024

Add autoplay toggle to the video player #5866

Open

1 task

absidue added the DO NOT MERGE UNTIL AFTER RELEASE Do not merge before the next release as this is not a bug fix label Oct 16, 2024

Display 'Option' instead of 'Alt' for Mac users

16693ea

kommunarr dismissed stale reviews from PikachuEXE and efb4f5ff-1298-471a-8973-3d47447115dc via 16693ea October 20, 2024 18:32

Use Mac icons in keyboard shortcuts for Macs

cf263fb

Update Mac arrow icon choice

d642682

Update KeyboardShortcuts constant organization in preparation for key…

348100d

…board shorcut modal

ChunkyProgrammer removed the DO NOT MERGE UNTIL AFTER RELEASE Do not merge before the next release as this is not a bug fix label Oct 27, 2024

efb4f5ff-1298-471a-8973-3d47447115dc approved these changes Oct 27, 2024

View reviewed changes

PikachuEXE approved these changes Oct 28, 2024

View reviewed changes

absidue reviewed Oct 28, 2024

View reviewed changes

efb4f5ff-1298-471a-8973-3d47447115dc requested changes Oct 28, 2024

View reviewed changes

github-actions bot added PR: merge conflicts / rebase needed and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels Nov 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add keyboard shortcuts to titles #5857

Add keyboard shortcuts to titles #5857

kommunarr commented Oct 13, 2024 •

edited

Loading

PikachuEXE commented Oct 14, 2024

kommunarr commented Oct 14, 2024 •

edited

Loading

efb4f5ff-1298-471a-8973-3d47447115dc left a comment

kommunarr commented Oct 14, 2024 •

edited

Loading

efb4f5ff-1298-471a-8973-3d47447115dc commented Oct 14, 2024 •

edited

Loading

kommunarr commented Oct 14, 2024

PikachuEXE left a comment

kommunarr commented Oct 20, 2024 •

edited

Loading

efb4f5ff-1298-471a-8973-3d47447115dc commented Oct 23, 2024

PikachuEXE commented Oct 23, 2024 •

edited

Loading

kommunarr commented Oct 23, 2024

kommunarr commented Oct 23, 2024

absidue Oct 28, 2024

absidue commented Oct 28, 2024 •

edited

Loading

kommunarr commented Oct 28, 2024

efb4f5ff-1298-471a-8973-3d47447115dc left a comment •

edited

Loading

kommunarr commented Oct 31, 2024

github-actions bot commented Nov 2, 2024

efb4f5ff-1298-471a-8973-3d47447115dc commented Nov 2, 2024

Add keyboard shortcuts to titles #5857

Are you sure you want to change the base?

Add keyboard shortcuts to titles #5857

Conversation

kommunarr commented Oct 13, 2024 • edited Loading

Add keyboard shortcuts to titles

Pull Request Type

Related issue

Description

Screenshots

Testing

Desktop

Additional context

PikachuEXE commented Oct 14, 2024

kommunarr commented Oct 14, 2024 • edited Loading

efb4f5ff-1298-471a-8973-3d47447115dc left a comment

Choose a reason for hiding this comment

kommunarr commented Oct 14, 2024 • edited Loading

efb4f5ff-1298-471a-8973-3d47447115dc commented Oct 14, 2024 • edited Loading

kommunarr commented Oct 14, 2024

PikachuEXE left a comment

Choose a reason for hiding this comment

kommunarr commented Oct 20, 2024 • edited Loading

efb4f5ff-1298-471a-8973-3d47447115dc commented Oct 23, 2024

PikachuEXE commented Oct 23, 2024 • edited Loading

kommunarr commented Oct 23, 2024

kommunarr commented Oct 23, 2024

absidue Oct 28, 2024

Choose a reason for hiding this comment

absidue commented Oct 28, 2024 • edited Loading

kommunarr commented Oct 28, 2024

efb4f5ff-1298-471a-8973-3d47447115dc left a comment • edited Loading

Choose a reason for hiding this comment

kommunarr commented Oct 31, 2024

github-actions bot commented Nov 2, 2024

efb4f5ff-1298-471a-8973-3d47447115dc commented Nov 2, 2024

kommunarr commented Oct 13, 2024 •

edited

Loading

kommunarr commented Oct 14, 2024 •

edited

Loading

kommunarr commented Oct 14, 2024 •

edited

Loading

efb4f5ff-1298-471a-8973-3d47447115dc commented Oct 14, 2024 •

edited

Loading

kommunarr commented Oct 20, 2024 •

edited

Loading

PikachuEXE commented Oct 23, 2024 •

edited

Loading

absidue commented Oct 28, 2024 •

edited

Loading

efb4f5ff-1298-471a-8973-3d47447115dc left a comment •

edited

Loading