Update app menu to add item for history #2870

PikachuEXE · 2022-11-16T06:26:22Z

Update app menu to add item for history

Pull Request Type

Bugfix
Feature Implementation
Documentation
Other

Related issue

#1869 Partially

Description

Update app menu to add item for history
with keyboard shortcut

Screenshots

Testing

Use new menu item to navigate to history view
Use new keyboard shortcut (Ctrl/Cmd + U) to navigate to history view

Desktop

OS: MacOS
OS Version: 12.6
FreeTube version: 2cc5c2e

Additional context

The namespace for translation entries used in history view needs to be updated to avoid key conflict

absidue · 2022-11-16T07:22:34Z

Pretty sure you didn't need to update the translation keys, as the menu items don't get translated anyway at the moment.
I think this should be two separate shortcuts:

cmd + Y on macOS as that's what the issue requestor wants
ctrl + H on Windows and Linux, as CTRL + Y is the keyboard shortcut for redo (the opposite of CTRL + Z to undo), CTRL + H is also the shortcut for opening the history on Firefox and Chromium browsers on Windows

PikachuEXE · 2022-11-16T07:35:29Z

Too complicated to have different keywords, I will just update it to Ctrl/Cmd + H

Update 1: Crap Cmd+H on MacOS = hide window zzzz
Update 2: Update to CmdOrCtrl+U

absidue · 2022-11-18T07:08:13Z

src/main/index.js

+          { type: 'separator' },
+          {
+            label: 'History',
+            accelerator: 'CmdOrCtrl+U',


It wouldn't be too hard to have a separate shortcut on macOS and have a meaningful one on Windows and Linux

Suggested change

accelerator: 'CmdOrCtrl+U',

accelerator: process.platform === 'darwin' ? 'Cmd+Y' : 'Ctrl+H',

Cmd+Z? Cmd+Y?

What's the standard shortcut for history in apps on macOS?

Cmd+Y I guess

Safari

Chrome

Updated but still need to be tested on windows

Thank you for implementing shortcuts! Once a menu item, a shortcut can be customised in macOS, so anything is good with me.

Once a menu item, a shortcut can be customised in macOS, so anything is good with me.

I didn't know that~

Yep, like this:

PikachuEXE · 2022-11-20T00:17:49Z

yarn.lock

@@ -7133,11 +7133,6 @@ sax@^1.1.3, sax@^1.2.4:
  resolved "https://registry.yarnpkg.com/sax/-/sax-1.2.4.tgz#2816234e2378bddc4e5354fab5caa895df7100d9"
  integrity sha512-NqVDv9TpANUjFm0N8uM5GxL36UgKi9/atZw+x7YFnQ8ckwFGKrl4xX4yWtrey3UJm5nP1kUbnYgLopqWNSRhWw==

-sax@~0.6.0:


I keep getting this on MacOS after running yarn install

PikachuEXE · 2022-11-21T02:36:04Z

.eslintrc.js

@@ -33,7 +33,7 @@ module.exports = {

  rules: {
    'space-before-function-paren': 'off',
-    'comma-dangle': ['error', 'never'],
+    'comma-dangle': ['error', 'only-multiline'],


I think multiline should allow (but not enforce) last comma

PikachuEXE · 2022-11-21T02:37:06Z

src/main/index.js

@@ -997,7 +997,23 @@ function runApp() {
          { role: 'zoomout' },
          { role: 'zoomout', accelerator: 'CmdOrCtrl+numsub', visible: false },
          { type: 'separator' },
-          { role: 'togglefullscreen' }
+          { role: 'togglefullscreen' },


Allowing last comma on multiline makes duplicating last code and modifying it much easier, also no more diff on N-1th line

ChunkyProgrammer · 2022-11-23T05:48:31Z

static/locales/en-US.yaml

@@ -119,7 +120,7 @@ User Playlists:
    it listed here
  Empty Search Message: There are no videos in this playlist that matches your search
  Search bar placeholder: Search in Playlist
-History:
+History Page:


Why rename this?
$t("History.History") would have gotten the history value before the update.

Also all locales that already have the History translated will need to update to "History Page" in order to not use the english translations

According to the comment # Webkit Menu Bar those entries are for Webkit Menu Bar and seems that they can only be placed on top level
I have no idea if that's true or not
Let me know?

The commit for that comment and many other entries are from a commit 2 years ago

Update 1: Found a tutorial about menu I18n

At the moment the menu doesn't get translated at all, so the translation change is not necessary yet, until someone decides to setup translations for the menu.

OK I will revert the change
Confusing!

PikachuEXE · 2022-11-24T01:01:43Z

translation namespace changes reverted

FreeTubeBot enabled auto-merge (squash) November 16, 2022 06:26

github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Nov 16, 2022

PikachuEXE requested review from absidue, efb4f5ff-1298-471a-8973-3d47447115dc and ChunkyProgrammer November 16, 2022 06:28

PikachuEXE mentioned this pull request Nov 16, 2022

~ Add doc about new shortcut FreeTubeApp/FreeTube-Docs#72

Merged

PikachuEXE force-pushed the feature/shortcuts/history branch from cc8fdfd to 5722ddb Compare November 16, 2022 07:44

absidue reviewed Nov 18, 2022

View reviewed changes

PikachuEXE force-pushed the feature/shortcuts/history branch from 5722ddb to 4607d06 Compare November 20, 2022 00:16

PikachuEXE commented Nov 20, 2022

View reviewed changes

absidue mentioned this pull request Nov 20, 2022

Update yarn lockfile #2878

Merged

1 task

PikachuEXE commented Nov 21, 2022

View reviewed changes

ChunkyProgrammer reviewed Nov 23, 2022

View reviewed changes

* Update app menu to add item for history

110c642

PikachuEXE force-pushed the feature/shortcuts/history branch from 4607d06 to 110c642 Compare November 24, 2022 01:01

absidue approved these changes Nov 24, 2022

View reviewed changes

ChunkyProgrammer approved these changes Nov 24, 2022

View reviewed changes

efb4f5ff-1298-471a-8973-3d47447115dc approved these changes Nov 24, 2022

View reviewed changes

FreeTubeBot merged commit 2ad4316 into FreeTubeApp:development Nov 24, 2022

github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Nov 24, 2022

PikachuEXE deleted the feature/shortcuts/history branch November 25, 2022 01:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update app menu to add item for history #2870

Update app menu to add item for history #2870

PikachuEXE commented Nov 16, 2022 •

edited

Loading

absidue commented Nov 16, 2022

PikachuEXE commented Nov 16, 2022 •

edited

Loading

absidue Nov 18, 2022 •

edited

Loading

PikachuEXE Nov 19, 2022

absidue Nov 19, 2022

PikachuEXE Nov 19, 2022

PikachuEXE Nov 20, 2022

lothar-cell Nov 21, 2022

PikachuEXE Nov 21, 2022

lothar-cell Nov 21, 2022

PikachuEXE Nov 20, 2022

PikachuEXE Nov 21, 2022

PikachuEXE Nov 21, 2022

ChunkyProgrammer Nov 23, 2022

PikachuEXE Nov 23, 2022 •

edited

Loading

absidue Nov 23, 2022

PikachuEXE Nov 24, 2022

PikachuEXE commented Nov 24, 2022

	accelerator: 'CmdOrCtrl+U',
	accelerator: process.platform === 'darwin' ? 'Cmd+Y' : 'Ctrl+H',

Update app menu to add item for history #2870

Update app menu to add item for history #2870

Conversation

PikachuEXE commented Nov 16, 2022 • edited Loading

Update app menu to add item for history

Pull Request Type

Related issue

Description

Screenshots

Testing

Desktop

Additional context

absidue commented Nov 16, 2022

PikachuEXE commented Nov 16, 2022 • edited Loading

absidue Nov 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PikachuEXE Nov 23, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PikachuEXE commented Nov 24, 2022

PikachuEXE commented Nov 16, 2022 •

edited

Loading

PikachuEXE commented Nov 16, 2022 •

edited

Loading

absidue Nov 18, 2022 •

edited

Loading

PikachuEXE Nov 23, 2022 •

edited

Loading