Skip to content

Get from title to top block wiithout mousing#50

Closed
clozach wants to merge 3 commits into
roam-unofficial:masterfrom
clozach:master
Closed

Get from title to top block wiithout mousing#50
clozach wants to merge 3 commits into
roam-unofficial:masterfrom
clozach:master

Conversation

@clozach

@clozach clozach commented May 9, 2020

Copy link
Copy Markdown

Addresses #50 . Feedback welcome!

clozach added 3 commits April 30, 2020 23:21
… the top

Problem: Moving from title editing to block editing currently requires the mouse.

Solution: This mod provides a means of getting from title to blocks without leaving the keyboard.
…at the top

Problem: When no blocks are currently selected, Roam requires a mouse click to begin editing.

Solution: This mod allows block editing without leaving the keyboard.
Comment thread src/manifest.json
"128": "assets/icon-128.png"
},
"content_security_policy": "script-src 'self'; object-src 'self'",
"content_security_policy": "script-src 'self' 'unsafe-eval'; object-src 'self'",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was actually removed for a reason, you need to revert that locally for development, but should not commit it

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops! Sorry for the misunderstanding. Will avoid in future.

const isExitingTitle = (ev: KeyboardEvent) => {
return ev.target.parentElement instanceof HTMLHeadingElement;
}
const isNoBlockSelected = (ev: KeyboardEvent) => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can use something like !Roam.getActiveRoamNode() to check for this

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. I'll give that a shot…assuming it's even worth continuing with this small feature given your note re: Cmd+Enter. Either way, I'll test it just to learn a little bit more. :)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, not sure if makes sense to include this given Cmd+Enter exists. I still appreciate you working on this though, and look forward to you building more things! ;)

document.addEventListener('keydown', ev => {
const enter = 'Enter';
const isExitingTitle = (ev: KeyboardEvent) => {
return ev.target.parentElement instanceof HTMLHeadingElement;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

given that you define this within the context of the handler, the event is already in the closure, so you don't have to pass it to the function.
in fact you can probably just use variable instead of function..

browser.runtime.onMessage.addListener((command) => dispatchMap.get(command)?.());

document.addEventListener('keydown', ev => {
const enter = 'Enter';

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if this constant adds much value tbh

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. I initially tried using the event.keyCode, which is much less clear in the condition check, but then neglected to inline the constant again when I switched to ev.key.

@Stvad

Stvad commented May 10, 2020

Copy link
Copy Markdown
Member

hmm, not sure since when, but apparently Roam has similar functionality natively now via Cmd+Enter when nothing is selected

@clozach clozach mentioned this pull request May 14, 2020
@clozach

clozach commented May 14, 2020

Copy link
Copy Markdown
Author

I decided to try this again from scratch, taking into account your feedback and making additional improvements. Closing in favor of #54. :)

@clozach clozach closed this May 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants