Keyboard shortcut modal #6306

kommunarr · 2024-12-08T01:33:40Z

Keyboard shortcut modal

Pull Request Type

Feature Implementation

Related issue

closes #2822

Description

Adds ~~Ctrl+Z~~ Shift+? keyboard shortcut (same as YT) + (for non- mobile device sizes only) button at the top of the settings page that opens/closes a modal containing all of the keyboard shortcuts available in FreeTube
Also tweaks ft-prompt logic to check for ft-icon-buttons as well, not just ft-buttons

Screenshots

Testing

Test Shift+? opens and closes the new modal from anywhere in the app
Test that close button is focused properly on open, even when another modal is already open
Check that modal content appears fine with different simulated device sizes
Play a video in fullscreen and/or fullwindow, press Shift+?, and ensure that FS and/or FW are successfully exited such that the keyboard shortcut modal is now visible

Desktop

OS: OpenSUSE
OS Version: TW

Additional context

I tried seeing if there was a simple programmatic way to check if the user is using a keyboard to determine whether to hide the button in Settings, but most solutions are messy and/or leave ample room for inaccuracy.

…at/keyboard-shortcut-modal

…c icons, capitalize non-special shortcut letters

absidue · 2024-12-08T08:24:34Z

I chose Ctrl+Z as a placeholder but ended up sticking with it. I'm fine with switching this to something else if there's a good reason to do so.

CTRL+Z is the standard keyboard shortcut for undo on Windows. We should probably use the same shortcut that YouTube does.

src/constants.js

src/renderer/components/FtKeyboardShortcutPrompt/FtKeyboardShortcutPrompt.vue

absidue · 2024-12-08T08:52:45Z

src/renderer/views/Settings/Settings.vue

+          <ft-button
+            :label="$t('Settings.Show Keyboard Shortcuts')"
+            @click="showKeyboardShortcutPrompt"
+          />


In your screenshot the button looks very out of place in its current position, but I can't think of anything better at the moment.

Fair call out, I didn't know what exactly to do with this one either. This was another adjustment I tried out. Open to this one or any other suggestions. @absidue

Maybe it could be under general setting with a keyboard icon added to the button? Or maybe a separate keyboard shortcuts "settings section" (so it wouldnt be using a modal popup and would just display all the shortcuts under the Keyboard Shortcuts section but we'd be able to update it to support #251 eventually).

If it's its own setting section, I do worry that people will think it's already modifiable. I can put it on its own row there as a button for now if that works, but it might still look out of place and not fit in as a "general setting".

Edit: I do love the keyboard icon idea and incorporated that now. Still unsure on how best to improve the placement.

src/constants.js

static/locales/en-US.yaml

Co-authored-by: absidue <[email protected]>

Change shortcut to be more accurate, remove duplicate action for closing current FreeTube window, update label for force reload

…reeTube into feat/keyboard-shortcut-modal

…6306

efb4f5ff-1298-471a-8973-3d47447115dc · 2024-12-09T00:36:29Z

go to a video
press fullwindow
press shift + ?
See that you cant leave video player anymore

github-actions · 2024-12-09T01:21:36Z

Conflicts have been resolved. A maintainer will review the pull request shortly.

PikachuEXE · 2024-12-09T05:53:59Z

Center it/full width in one column view? (mobile on should just get full width regardless

efb4f5ff-1298-471a-8973-3d47447115dc · 2024-12-16T23:24:08Z

Weird suggestion:

If a contributor adds a shortcut key than they also have to :

open up a PR to reflect that change in the docs repo
add it to the keyboard shortcut modal

Would it be possible to generate the contents of the shortcut modal based on the docs? That way if someone changes something in the docs it will automatically be reflected within the app. Pull latest list on startup just like the IV instances.

efb4f5ff-1298-471a-8973-3d47447115dc · 2024-12-16T23:29:26Z

Center it/full width in one column view? (mobile on should just get full width regardless

@PikachuEXE just tested is and it looks pretty good. Unsure if this is really needed

ChunkyProgrammer · 2024-12-16T23:34:41Z

Weird suggestion:

If a contributor adds a shortcut key than they also have to :

open up a PR to reflect that change in the docs repo

add it to the keyboard shortcut modal

Would it be possible to generate the contents of the shortcut modal based on the docs? That way if someone changes something in the docs it will automatically be reflected within the app. Pull latest list on startup just like the IV instances.

I don't think we should do that. Invidious instance list is from a public api that we dont control and can be updated without a new version of FreeTube being installed. If we get the modal from the docs repo then either the modal will be incorrect for nightlies or will be incorrect for the release.

Another option is we could update the docs repo's keyboard shortcuts section to mention that you can now view the keyboard shortcuts in the app, show how to view the shortcuts and mention how that page might be out of date.

kommunarr · 2024-12-16T23:44:08Z

Weird suggestion:

If a contributor adds a shortcut key than they also have to :
* open up a PR to reflect that change in the docs repo

* add it to the keyboard shortcut modal
Would it be possible to generate the contents of the shortcut modal based on the docs? That way if someone changes something in the docs it will automatically be reflected within the app. Pull latest list on startup just like the IV instances.

That's an interesting idea. The two main approaches I can think of would be parsing the HTML directly (which might have different formatting, preambles, etc. than we would prefer for an in-app modal) or getting it from a JSON file of shortcuts (like we do with the Invidious instances). The main issue with the latter is that we'd have to either update the JSON file as well for each change (thus being the same amount of effort of two changes per new/modified shortcut) or convert the docs page to grabbing the data dynamically from the JSON file as well, which would be shaky and might be hard to section & format. So I don't think there's a solution at the moment that would guarantee a lower amount of net effort.

Edit: Whoops, just updated to see a similar comment by @ChunkyProgrammer.

efb4f5ff-1298-471a-8973-3d47447115dc · 2024-12-16T23:49:09Z

Should code comments be added to the shortcuts section so people are aware they also have to change it in the modal when they are adding/changing shortcuts?

PikachuEXE · 2024-12-17T00:19:42Z

The latest code comment doesn't seem to mention the modal

…tation should be

PikachuEXE · 2024-12-24T03:13:38Z

Implemented #5966 but it depends on this
Reminder for myself to open PR once this merged?

src/renderer/components/FtKeyboardShortcutPrompt/FtKeyboardShortcutPrompt.vue

absidue · 2024-12-26T12:14:47Z

src/renderer/components/FtKeyboardShortcutPrompt/FtKeyboardShortcutPrompt.css

+.shortcut {
+  font-family: monospace;
+  display: table-cell;
+  vertical-align: middle;
+  text-transform: capitalize;
+}


Non-blocking: We should probably use the <kbd> HTML tag to inform the browser that they are keyboard keys, instead of setting it to monospace with CSS. https://developer.mozilla.org/en-US/docs/Web/HTML/Element/kbd

The reason I am tagging this as non-blocking is that it would require changes to the way we convert the keyboard shortcuts in getLocalizedShortcut as each key would need to be wrapped with that HTML tag.

I do agree with this point in the abstract, although for this particular element type, the benefit is quite small.

absidue · 2024-12-26T12:21:45Z

src/renderer/components/FtKeyboardShortcutPrompt/FtKeyboardShortcutPrompt.vue

+    ['SHOW_SHORTCUTS', t('KeyboardShortcutPrompt.Show Keyboard Shortcuts')],
+    ['HISTORY_BACKWARD', t('KeyboardShortcutPrompt.History Backward')],
+    ['HISTORY_FORWARD', t('KeyboardShortcutPrompt.History Forward')],
+    ['FULLSCREEN', t('KeyboardShortcutPrompt.Fullscreen')],
+    ['NAVIGATE_TO_SETTINGS', t('KeyboardShortcutPrompt.Navigate to Settings')],
+    (
+      isMac
+        ? ['NAVIGATE_TO_HISTORY_MAC', t('KeyboardShortcutPrompt.Navigate to History')]
+        : ['NAVIGATE_TO_HISTORY', t('KeyboardShortcutPrompt.Navigate to History')]
+    ),
+    ['NEW_WINDOW', t('KeyboardShortcutPrompt.New Window')],
+    ['MINIMIZE_WINDOW', t('KeyboardShortcutPrompt.Minimize Window')],
+    ['CLOSE_WINDOW', t('KeyboardShortcutPrompt.Close Window')],
+    ['RESTART_WINDOW', t('KeyboardShortcutPrompt.Restart Window')],
+    ['FORCE_RESTART_WINDOW', t('KeyboardShortcutPrompt.Force Restart Window')],
+    ['TOGGLE_DEVTOOLS', t('KeyboardShortcutPrompt.Toggle Developer Tools')],
+    ['RESET_ZOOM', t('KeyboardShortcutPrompt.Reset Zoom')],
+    ['ZOOM_IN', t('KeyboardShortcutPrompt.Zoom In')],
+    ['ZOOM_OUT', t('KeyboardShortcutPrompt.Zoom Out')],
+    ['FOCUS_SEARCH', t('KeyboardShortcutPrompt.Focus Search')],
+    ['SEARCH_IN_NEW_WINDOW', t('KeyboardShortcutPrompt.Search in New Window')],
+
+    ['REFRESH', t('KeyboardShortcutPrompt.Refresh')],
+    ['FOCUS_SECONDARY_SEARCH', t('KeyboardShortcutPrompt.Focus Secondary Search')],
+
+    ['CAPTIONS', t('KeyboardShortcutPrompt.Captions')],
+    ['THEATRE_MODE', t('KeyboardShortcutPrompt.Theatre Mode')],
+    ['FULLSCREEN', t('KeyboardShortcutPrompt.Fullscreen')],
+    ['FULLWINDOW', t('KeyboardShortcutPrompt.Full Window')],
+    ['PICTURE_IN_PICTURE', t('KeyboardShortcutPrompt.Picture in Picture')],
+    ['MUTE', t('KeyboardShortcutPrompt.Mute')],
+    ['VOLUME_UP', t('KeyboardShortcutPrompt.Volume Up')],
+    ['VOLUME_DOWN', t('KeyboardShortcutPrompt.Volume Down')],
+    ['TAKE_SCREENSHOT', t('KeyboardShortcutPrompt.Take Screenshot')],
+    ['STATS', t('KeyboardShortcutPrompt.Stats')],
+
+    ['PLAY', t('KeyboardShortcutPrompt.Play')],
+    ['LARGE_REWIND', t('KeyboardShortcutPrompt.Large Rewind')],
+    ['LARGE_FAST_FORWARD', t('KeyboardShortcutPrompt.Large Fast Forward')],
+    ['SMALL_REWIND', t('KeyboardShortcutPrompt.Small Rewind')],
+    ['SMALL_FAST_FORWARD', t('KeyboardShortcutPrompt.Small Fast Forward')],
+    ['DECREASE_VIDEO_SPEED', t('KeyboardShortcutPrompt.Decrease Video Speed')],
+    ['INCREASE_VIDEO_SPEED', t('KeyboardShortcutPrompt.Increase Video Speed')],
+    ['SKIP_N_TENTHS', t('KeyboardShortcutPrompt.Skip by Tenths')],
+    ['LAST_CHAPTER', t('KeyboardShortcutPrompt.Last Chapter')],
+    ['NEXT_CHAPTER', t('KeyboardShortcutPrompt.Next Chapter')],
+    ['LAST_FRAME', t('KeyboardShortcutPrompt.Last Frame')],
+    ['NEXT_FRAME', t('KeyboardShortcutPrompt.Next Frame')],


Note to future self: Look into if there is a way to do this, without having to having to use strings as the keys, to reduce the bundle size (also forces terser to bail-out of most optimisations on the KeyboardShortcuts constant e.g. mangling the names and hoisting them out of the object). Currently this pull request adds about 7921 bytes to the bundle size.

As a note, this representation might change a bit if we implement #251, but I imagine we'll keep the keys around. I don't have any intention on implementing it currently, but my thought would be we have each action has a unique name, each action+name is mapped to a unique shortcut combo, and we replace a lot of our keydown logic with a combo-reading util function that accepts a list of accepted names as arguments. The localization key in that case could either come from putting the name through a de-snake-case-ifier or as another property on the constant object.

…6306 (#154)

* Add keyboard shortcut modal * Temp * Add keyboard shortcut modal shortcut entry, shift & enter labels + Mac icons, capitalize non-special shortcut letters * Use usei18n instead of i18n.t * Improve keyboard shortcut button formatting * Update prompt logic to focus ft-icon-buttons as well * Improve mobile styling * Adjust v-for key to reflect new template structure * Apply suggestions from code review Co-authored-by: absidue <[email protected]> * Implement review suggestions Change shortcut to be more accurate, remove duplicate action for closing current FreeTube window, update label for force reload * Change shortcut to YouTube's SHIFT+? * Use keyboard icon for button, and check for usingElectron in showing it * Change ctrl+R shortcuts to use new labelling * Exit FS and/or FW when keyboard shortcut modal is opened * Add code comment clarifying where parallel change to shortcut documentation should be * Implement changes from review --------- Co-authored-by: absidue <[email protected]>

kommunarr added 8 commits October 23, 2024 17:12

Add keyboard shortcut modal

b248cac

Temp

6205134

Merge branch 'development' of github.com:FreeTubeApp/FreeTube into fe…

b2b37ba

…at/keyboard-shortcut-modal

Add keyboard shortcut modal shortcut entry, shift & enter labels + Ma…

2045f25

…c icons, capitalize non-special shortcut letters

Use usei18n instead of i18n.t

c6da141

Improve keyboard shortcut button formatting

d04ee58

Update prompt logic to focus ft-icon-buttons as well

956c376

Improve mobile styling

b66eda1

kommunarr added the PR: waiting for review For PRs that are complete, tested, and ready for review label Dec 8, 2024

FreeTubeBot enabled auto-merge (squash) December 8, 2024 01:33

Adjust v-for key to reflect new template structure

6297857

kommunarr requested review from PikachuEXE, absidue, efb4f5ff-1298-471a-8973-3d47447115dc, ChunkyProgrammer and MarmadileManteater December 8, 2024 01:46

absidue reviewed Dec 8, 2024

View reviewed changes

kommunarr and others added 3 commits December 8, 2024 14:37

Apply suggestions from code review

1f2df14

Co-authored-by: absidue <[email protected]>

Implement review suggestions

31f93ae

Change shortcut to be more accurate, remove duplicate action for closing current FreeTube window, update label for force reload

Merge branch 'feat/keyboard-shortcut-modal' of github.com:kommunarr/F…

8476c4a

…reeTube into feat/keyboard-shortcut-modal

kommunarr force-pushed the feat/keyboard-shortcut-modal branch from 5469153 to 8476c4a Compare December 8, 2024 14:53

kommunarr added a commit to kommunarr/FreeTube-Docs that referenced this pull request Dec 8, 2024

Modify keyboard shortcuts to reflect changes in FreeTubeApp/FreeTube#…

d01d80f

…6306

Change shortcut to YouTube's SHIFT+?

1118787

kommunarr added a commit to kommunarr/FreeTube-Docs that referenced this pull request Dec 8, 2024

Modify keyboard shortcuts to reflect changes in FreeTubeApp/FreeTube#…

df8f4a8

…6306

kommunarr mentioned this pull request Dec 8, 2024

Modify keyboard shortcuts FreeTubeApp/FreeTube-Docs#154

Merged

kommunarr added 2 commits December 8, 2024 09:58

Use keyboard icon for button, and check for usingElectron in showing it

5be2bec

Change ctrl+R shortcuts to use new labelling

e88ad8c

kommunarr added a commit to kommunarr/FreeTube-Docs that referenced this pull request Dec 8, 2024

Modify keyboard shortcuts to reflect changes in FreeTubeApp/FreeTube#…

8eb29cd

…6306

github-actions bot removed the PR: merge conflicts / rebase needed label Dec 9, 2024

efb4f5ff-1298-471a-8973-3d47447115dc added the PR: waiting for review For PRs that are complete, tested, and ready for review label Dec 11, 2024

kommunarr force-pushed the feat/keyboard-shortcut-modal branch from 00eff2a to 31349ab Compare December 17, 2024 00:34

Add code comment clarifying where parallel change to shortcut documen…

0126970

…tation should be

kommunarr force-pushed the feat/keyboard-shortcut-modal branch from 31349ab to 0126970 Compare December 17, 2024 00:48

efb4f5ff-1298-471a-8973-3d47447115dc previously approved these changes Dec 23, 2024

View reviewed changes

PikachuEXE previously approved these changes Dec 23, 2024

View reviewed changes

PikachuEXE requested a review from absidue December 25, 2024 10:25

absidue reviewed Dec 26, 2024

View reviewed changes

Implement changes from review

2e36a7c

kommunarr dismissed stale reviews from PikachuEXE and efb4f5ff-1298-471a-8973-3d47447115dc via 2e36a7c December 26, 2024 14:48

kommunarr requested a review from absidue December 27, 2024 03:11

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

View reviewed changes

PikachuEXE approved these changes Dec 27, 2024

View reviewed changes

absidue approved these changes Dec 27, 2024

View reviewed changes

FreeTubeBot merged commit 6693654 into FreeTubeApp:development Dec 27, 2024
5 checks passed

github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Dec 27, 2024

FreeTubeBot pushed a commit to FreeTubeApp/FreeTube-Docs that referenced this pull request Jan 11, 2025

Modify keyboard shortcuts to reflect changes in FreeTubeApp/FreeTube#…

40ac18e

…6306 (#154)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Keyboard shortcut modal #6306

Keyboard shortcut modal #6306

kommunarr commented Dec 8, 2024 •

edited

Loading

absidue commented Dec 8, 2024

absidue Dec 8, 2024

kommunarr Dec 8, 2024

ChunkyProgrammer Dec 8, 2024

kommunarr Dec 8, 2024 •

edited

Loading

efb4f5ff-1298-471a-8973-3d47447115dc commented Dec 9, 2024

github-actions bot commented Dec 9, 2024

PikachuEXE commented Dec 9, 2024

efb4f5ff-1298-471a-8973-3d47447115dc commented Dec 16, 2024

efb4f5ff-1298-471a-8973-3d47447115dc commented Dec 16, 2024

ChunkyProgrammer commented Dec 16, 2024

kommunarr commented Dec 16, 2024 •

edited

Loading

efb4f5ff-1298-471a-8973-3d47447115dc commented Dec 16, 2024

PikachuEXE commented Dec 17, 2024

PikachuEXE commented Dec 24, 2024

absidue Dec 26, 2024

kommunarr Dec 26, 2024

absidue Dec 26, 2024 •

edited

Loading

kommunarr Dec 26, 2024

Keyboard shortcut modal #6306

Keyboard shortcut modal #6306

Conversation

kommunarr commented Dec 8, 2024 • edited Loading

Keyboard shortcut modal

Pull Request Type

Related issue

Description

Screenshots

Testing

Desktop

Additional context

absidue commented Dec 8, 2024

absidue Dec 8, 2024

Choose a reason for hiding this comment

kommunarr Dec 8, 2024

Choose a reason for hiding this comment

ChunkyProgrammer Dec 8, 2024

Choose a reason for hiding this comment

kommunarr Dec 8, 2024 • edited Loading

Choose a reason for hiding this comment

efb4f5ff-1298-471a-8973-3d47447115dc commented Dec 9, 2024

github-actions bot commented Dec 9, 2024

PikachuEXE commented Dec 9, 2024

efb4f5ff-1298-471a-8973-3d47447115dc commented Dec 16, 2024

efb4f5ff-1298-471a-8973-3d47447115dc commented Dec 16, 2024

ChunkyProgrammer commented Dec 16, 2024

kommunarr commented Dec 16, 2024 • edited Loading

efb4f5ff-1298-471a-8973-3d47447115dc commented Dec 16, 2024

PikachuEXE commented Dec 17, 2024

PikachuEXE commented Dec 24, 2024

absidue Dec 26, 2024

Choose a reason for hiding this comment

kommunarr Dec 26, 2024

Choose a reason for hiding this comment

absidue Dec 26, 2024 • edited Loading

Choose a reason for hiding this comment

kommunarr Dec 26, 2024

Choose a reason for hiding this comment

kommunarr commented Dec 8, 2024 •

edited

Loading

kommunarr Dec 8, 2024 •

edited

Loading

kommunarr commented Dec 16, 2024 •

edited

Loading

absidue Dec 26, 2024 •

edited

Loading