WIP: Shortcuts streamline & help page

[Denperidge]

Hi! Due to the 1st part of the code being done (streamline) and the second (help page) solely delayed by my troubles getting rid of Page Not Found in React, I thought I'd open the PR as a draft to possibly get feedback on the work so far.

Once finished, this will close #1143. Help in getting the page showing would be appreciated too!

Shortcut streamlining (done)

• (Changed) KeybindingsMap is now generated from a simpler, less boilerplate-heavy format
  All keybindings have been tested manually since conversion
• (Description) Adapted shortcuts.ts descriptions to their equivalents from the docs
  https://github.com/fossar/selfoss/blob/81187a39a1b1db0e2d48fb0fccad949ca771a635/docs/content/docs/usage/shortcuts.md
  • Exception: "item" is replaced by "entry" for consistency
  • Exception: Renamed throws to throw to previous / throw to next
• (Keycombo readable name) Adapted arrow left & arrow right to the symbol used in docs
• (Refactor) Due to being closely related, moved ctrl+m to be next to m & j to be next so space

Help page (wip)

• (Changed) move shortcut table styling to shared scss file, created HelpS…](bb2d08c) (done)
• (Addition) Create ? shortcut (wip)
• (Addition) Create help page (wip)

[netlify]

✅ Deploy Preview for selfoss canceled.

Name	Link
🔨 Latest commit	23cb3dd
🔍 Latest deploy log	https://app.netlify.com/projects/selfoss/deploys/6a822164e1e1fe000869ad05

[jtojnar]

Thanks. This looks promising, will take a deeper look tomorrow.

Might be nice to go even further and re-use the shortcuts in the website docs through a JSON file. Thought that would require going through an indirection of mapping action name (string) to a function and using the action names in the JSON.

On a side note, would it be possible to extract the re-ordering (and nothing else) into a separate commit for easier review. As it is currently, it is difficult to see the other changes.

[Denperidge]

Might be nice to go even further and re-use the shortcuts in the website docs through a JSON file. Thought that would require going through an indirection of mapping action name (string) to a function and using the action names in the JSON.

That was my plan for after this PR! Would it be an okay workaround to have the JSON be auto-generated? Have a function in shortcuts.ts that creates the json, for example. This way no string mappings have to be made for the actions, and the typed typescript remains the source of truth. But I'll adapt to the JSON-first structure if preferred!

On a side note, would it be possible to extract the re-ordering (and nothing else) into a separate commit for easier review. As it is currently, it is difficult to see the other changes.

With the re-ordering, I presume you mean the (Refactor) Due to being closely related, moved ctrl+m to be next to m & j to be next so space part? Let me know what you mean, and I'll do my best to extract it! (Although admittedly, I haven't extracted specific chunks of a file in a commit a lot, so I'll send you a separate branch before force pushing into here)

[jtojnar]

Would it be an okay workaround to have the JSON be auto-generated?

AFAIK, zola does not support running external commands so either the file would have to be committed in the repo (and have CI check that it did not get out of sync), or we would have to switch to JS based static site generator like Astro. Both of which I would rather avoid.

This way no string mappings have to be made for the actions, and the typed typescript remains the source of truth.

I am thinking we might want to have actions mapping in the longer term anyway, to allow shortcut remapping. (#1611)

Though maybe I just would not bother with this and replace the shortcuts website page with suggestion to check the list in the app. Perhaps later with a link to a demo page. (#1364)

With the re-ordering, I presume you mean the (Refactor) Due to being closely related, moved ctrl+m to be next to m & j to be next so space part?

Yes. Mostly I want the individual commit diffs to be clear what each change is doing to make reviews by me and security reviews by our users easier. When multiple changes are mixed, it is easy to miss something so it requires more attention (perhaps recreating what the commit message is describing and then comparing the diffs).

Unfortunately, it is much harder to split commit than to combine multiple small ones, especially when the changes are overlapping. In this case, I do not see other option than to recreate the PR from scratch with more atomic commits. I can do that if you do not want to bother since I would have to separate the changes for review anyway.

[Denperidge]

AFAIK, zola does not support running external commands so either the file would have to be committed in the repo (and have CI check that it did not get out of sync), or we would have to switch to JS based static site generator like Astro. Both of which I would rather avoid.
[...]
I am thinking we might want to have actions mapping in the longer term anyway, to allow shortcut remapping. (#1611)

Great reasoning! Do I rebase my PR onto #1610?

Though maybe I just would not bother with this and replace the shortcuts website page with suggestion to check the list in the app. Perhaps later with a link to a demo page. (#1364)

I think it's good to have it on the docs!

• For search engines queries. It's my top result for selfoss shortcuts, and fifth for selfoss keybindings. If someone doesn't know where to look in the app/demo, they tend to gravitate to the good old world wide web.
• Allows people to see the current shortcut bindings and what they're generally capable of before installing; great if someone is looking for specific workflows
• And, while this is also due to my personal feelings towards documentation, I do think it's good to have the documentations contain more than less information. The shortcut overview is succinct, and with our PR's, way easier to maintain

Of course, you maintain the project; feel free to disagree

Yes. Mostly I want the individual commit diffs to be clear what each change is doing to make reviews by me and security reviews by our users easier. When multiple changes are mixed, it is easy to miss something so it requires more attention (perhaps recreating what the commit message is describing and then comparing the diffs).

Noted! I'll try to split it up, and keep note to make smaller commits

Unfortunately, it is much harder to split commit than to combine multiple small ones, especially when the changes are overlapping. In this case, I do not see other option than to recreate the PR from scratch with more atomic commits. I can do that if you do not want to bother since I would have to separate the changes for review anyway.

I'll give it a shot! If I'd ask your help for everything, its getting the help page to display. But if you don't have time dw, i'll give it another shot after doing some other tasks

[Denperidge]

Just finished the reformat branch! Could you check it out? If that style of commits is good, I'll modify the WIP to it too https://github.com/Denperidge/contrib-selfoss/tree/keybindings-reform

[jtojnar]

Great reasoning! Do I rebase my PR onto #1610?

We can handle that separately later. After all, having the shortcuts duplicated is the status quo.

I think it's good to have it on the docs!

Yeah, it probably does not hurt anything. I just wish zola would allow loading stuff from parent directories.

Just finished the reformat branch

Looks good, thanks.

getting the help page to display.

I would use the dialog element:

diff --git a/client/js/shortcuts.ts b/client/js/shortcuts.ts
index 92c33145..c3e598af 100644
--- a/client/js/shortcuts.ts
+++ b/client/js/shortcuts.ts
@@ -1,6 +1,7 @@
 import { tinykeys } from 'tinykeys';
 import selfoss from './selfoss-base';
 import { Direction } from './helpers/navigation';
+import { RefObject } from 'react';
 
 type KeyboardEventHandler = (event: KeyboardEvent) => void;
 
@@ -23,7 +24,7 @@ function ignoreWhenInteracting(
 /**
  * Set up shortcuts on document.
  */
-export default function makeShortcuts(): () => void {
+export default function makeShortcuts(shortcutsDialog: RefObject<HTMLDialogElement>): () => void {
     return tinykeys(window, {
         // 'space': next article
         Space: ignoreWhenInteracting((event: KeyboardEvent): void => {
@@ -162,5 +163,12 @@ export default function makeShortcuts(): () => void {
                 .querySelector<HTMLAnchorElement>('#nav-filter-starred')
                 .click();
         }),
+
+        // h works, not sure ? does on all layouts
+        '?': ignoreWhenInteracting((event: KeyboardEvent): void => {
+            event.preventDefault();
+            // TODO: disable shortcuts while active like with lightbox
+            shortcutsDialog.current.showModal();
+        }),
     });
 }
diff --git a/client/js/templates/App.tsx b/client/js/templates/App.tsx
index b07821ff..34370bb7 100644
--- a/client/js/templates/App.tsx
+++ b/client/js/templates/App.tsx
@@ -8,6 +8,7 @@ import React, {
     useMemo,
     useState,
     MouseEvent,
+    useRef,
 } from 'react';
 import {
     BrowserRouter as Router,
@@ -246,6 +247,7 @@ function PureApp(props: PureAppProps): React.JSX.Element {
     } = props;
 
     const [navExpanded, setNavExpanded] = useState(false);
+    const shortcutsDialog = useRef<HTMLDialogElement>(null);
     const smartphone = useIsSmartphone();
     const offlineEnabled = useListenableValue(selfoss.db.enableOffline);
     const [entriesPage, setEntriesPage] = useState(null);
@@ -253,7 +255,7 @@ function PureApp(props: PureAppProps): React.JSX.Element {
 
     useEffect(() => {
         // init shortcut handler
-        const destroyShortcuts = makeShortcuts();
+        const destroyShortcuts = makeShortcuts(shortcutsDialog);
 
         return () => {
             destroyShortcuts();
@@ -480,6 +482,10 @@ function PureApp(props: PureAppProps): React.JSX.Element {
                                     />
                                     <Route path="*" element={<NotFound />} />
                                 </Routes>
+
+                                <dialog ref={shortcutsDialog}>
+                                    shortcuts {/*<Shortcuts />*/}
+                                </dialog>
                             </div>
                         </CheckAuthorization>
                     }

[Denperidge]

Well, the force push biffed it. Re-opening in a second!