-
-
Notifications
You must be signed in to change notification settings - Fork 43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Get from title to top block wiithout mousing #50
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,20 @@ const dispatchMap = new Map([ | |
|
||
browser.runtime.onMessage.addListener((command) => dispatchMap.get(command)?.()); | ||
|
||
document.addEventListener('keydown', ev => { | ||
const enter = 'Enter'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not sure if this constant adds much value tbh There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
const isExitingTitle = (ev: KeyboardEvent) => { | ||
return ev.target.parentElement instanceof HTMLHeadingElement; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
} | ||
const isNoBlockSelected = (ev: KeyboardEvent) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you can use something like There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, not sure if makes sense to include this given |
||
return ev.target instanceof HTMLBodyElement; | ||
} | ||
|
||
if (ev.key === enter && (isExitingTitle(ev) || isNoBlockSelected(ev))) { | ||
Roam.createBlockAtTop(); | ||
} | ||
}); | ||
|
||
document.addEventListener('keyup', ev => { | ||
if (ev.key === guard) replaceFuzzyDate(); | ||
}); |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.