migrate commands to command palette (Closes #389)

[mdroidian]

Let me know if I'm on the right track 👍 @dvargas92495

components PR: https://github.com/dvargas92495/roamjs-components/pull/3

[mdroidian]

Also, the docs seem to strongly recommend against setting a default shortcut. Do we still want to go ahead with that?

[mdroidian]

Struggling with Typescript definition and console error with the decorators.tsx: https://github.com/dvargas92495/roamjs-workbench/pull/391/commits/d4d78f7a5fb634242100410e54ea274e02348787

Typescript error:

Type '{ settings: 
{ get: (k: string) => unknown; getAll: () => Record<string, unknown>; 
panel: { create: (c: PanelConfig) => void; }; 
set: (k: string, v: unknown) => Promise<void>; }; 
ui: { commandPalette?: { ...; }; }; }' 
is not assignable to type 'never'.ts(2322)

Console error:

Uncaught TypeError: parseRoamMarked is not a function
    at callback (decorators.tsx:482:27)
    at callback (createHashtagObserver.js:12:13)
    at Array.forEach (<anonymous>)
    at createHTMLObserver (createHTMLObserver.js:8:27)
    at createHashtagObserver (createHashtagObserver.js:5:94)
    at toggleFeature (decorators.tsx:473:119)
    at onClick (decorators.tsx:335:7)
...

[mdroidian]

Console error:

Uncaught TypeError: parseRoamMarked is not a function
    at callback (decorators.tsx:482:27)
    at callback (createHashtagObserver.js:12:13)
    at Array.forEach (<anonymous>)
    at createHTMLObserver (createHTMLObserver.js:8:27)
    at createHashtagObserver (createHashtagObserver.js:5:94)
    at toggleFeature (decorators.tsx:473:119)
    at onClick (decorators.tsx:335:7)
...

Maybe this isn't actually from the code change. Looks like something similar exists in the current version.

helpers.ts:120 Uncaught TypeError: i is not a function
    at callback (6a693c5a-69ce-4ef4-842d-c096939dbf20:2:1646319)
    at callback (6a693c5a-69ce-4ef4-842d-c096939dbf20:2:1245300)
    at Array.forEach (<anonymous>)
    at t.default (6a693c5a-69ce-4ef4-842d-c096939dbf20:2:1244712)
    at t.Z (6a693c5a-69ce-4ef4-842d-c096939dbf20:2:1245189)
    at _n (6a693c5a-69ce-4ef4-842d-c096939dbf20:2:1646081)
    at onClick (6a693c5a-69ce-4ef4-842d-c096939dbf20:2:1644114)
...

Steps to replicate:

1. Turn off all decorations
2. Disable Workbench
3. Reload Graph
4. Enable Workbench
5. Toggle all decorations. Save.

[dvargas92495]

Yea this looks like a current bug from when the context decorator is enabled - I'd try to ignore this for now as its a separate issue

[mdroidian]

for https://github.com/dvargas92495/roamjs-workbench/pull/391/commits/b21a05201d06598eddc52e26ad45467fc647fdd6

Removed the toggle right/left sidebar, as those are built into Roam now.

Also, let me know how you feel about code formatting

I went with that because if felt more readable to me than the 200+ lines from the Prettier extension.

[mdroidian]

Issues found with current jumpNav (Hot Keys)

• X - Expand/Collapse block tree to a certain level, specified by the following numeric key press
  • number press adds number to block

And these appear to not work:

• S - Add shortcut to page to left sidebar
• z - Toggle Unlinked Refs
• y - Toggle Queries
• b - Jump to the bottom of page
  • jumps to top of page
• q - Toggle View type
  • toggles to document, but not back to bullet

[mdroidian]

Well here's an unfortunate side effect of addCommand When a command is invoke that uses the command palette sets focus of the block, then the cursor disappears. Using a hot key, this problem does not occur.

Added timeout here: https://github.com/dvargas92495/roamjs-workbench/pull/391/commits/4629860ad8e6ec4911259fe7c6175b1c3446b16c

But I feel it is bordering an unreasonable length of time.

[dvargas92495]

I went with that because if felt more readable to me than the 200+ lines from the Prettier extension.

The useful part of the prettier extension is the standardization, not necessarily the decisions it comes with by default. The problem with making exceptions is that someone else working on the team will eventually edit the file, hit save, then prettier will auto format.

If you disagree with prettier defaults, the scalable solution would be to change the prettier rules, so that it gets formatted automatically. If it's unable to be encoded, then the opinion is subjective, which means it will inevitably be overwritten by somebody else's subjective experience, and now time on the PR is being spent on code formatting instead of code content.

Added timeout here

Man the problem here is I think this API from Roam is actually not very reliable - ideally we don't need a setTimeout at all. This should be fine for now

Gonna start reviewing PR now

[mdroidian]

I went with that because if felt more readable to me than the 200+ lines from the Prettier extension.

The useful part of the prettier extension is the standardization, not necessarily the decisions it comes with by default. The problem with making exceptions is that someone else working on the team will eventually edit the file, hit save, then prettier will auto format.

If you disagree with prettier defaults, the scalable solution would be to change the prettier rules, so that it gets formatted automatically. If it's unable to be encoded, then the opinion is subjective, which means it will inevitably be overwritten by somebody else's subjective experience, and now time on the PR is being spent on code formatting instead of code content.

This makes sense, but if someone else were to contribute to this code, how would they be made aware of the prettier / format on save convention?

[mdroidian]

Speaking of which, I'm not using the "on save feature". Should I be? I just an uneasy feeling when I go to edit a few lines by the commit shows quite a few more changes than I made.

[dvargas92495]

how would they be made aware of the prettier / format on save convention?

This is somewhat implicit by seeing prettier in a repo in the industry at this point. Sortof like if you see a testing library you can assume that the project is running tests in Github in some capacity and they should all be passing before merging

Speaking of which, I'm not using the "on save feature". Should I be?

I highly recommend it, it's life changing :+1:

I just an uneasy feeling when I go to edit a few lines by the commit shows quite a few more changes than I made.

That's okay bc of two reasons:

1. Prettier only does whitespace changes - it does not affect semantic reasoning
2. Because of 1, you can review PRs by checking "hide whitespace changes" to ignore those changes when reviewing

[mdroidian]

A few notes / questions on default hotkeys:

1. DNP Jump Date Forward / Back conflict with the inbuild Roam Cycle Version

2. alt-shift-p doesn't work for me (in Privacy Mode)

As a matter of fact, I can't set any hotkey using the UI to alt-shift-p. I did check another program, however, and alt-shift-p worked, so it doesn't seem to be my system that is the issue. Could you check if it works for you?

4. How do you want to handle the default hotkeys for Hot Keys (jumpNav.tsx)?

The docs say

All the hot keys in this module are initiated with a modifier + j + another character. So Alt+j, Ctrl+j and CMD+j are all equivalent in this context. The final character, pressed in succession after modifier + j, is what determines the action taken.

I don't believe we can set more than one hotkey as a default.

5. I believe the code in weekly-notes.ts actually allowed for both hotkeys because .isIOS is "true if client is iphone, ipad or ipod"

I coded it to stay in line with the docs

To jump to this week's page, hit Alt+w or Ctrl+Shift+W (Mac Only).

[dvargas92495]

DNP Jump Date Forward / Back conflict with the inbuild Roam Cycle Version

How do you want to handle the default hotkeys for Hot Keys (jumpNav.tsx)?

Ok so I'm going to double back and what I said before and make an exception for this and jumpnav. Let's remove the defaults, and be sure to make announcements in #roam-js and twitter, emphasizing that the reason why we removed the defaults here are so that users could configure them in Roam's hot key panel.

alt-shift-p doesn't work for me (in Privacy Mode)

Works for me - Sometimes chrome extensions could conflict with hot keys

[mdroidian]

It might be prudent to not merge this until the docs have been updated. Also, I have another idea to toss your way on Monday that relates to this.

Works for me - Sometimes chrome extensions could conflict with hot keys

🤦‍♂️ That was it!