Skip to content
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

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"icons": {
"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
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
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.

"permissions": [
"activeTab",
"alarms",
Expand Down
14 changes: 14 additions & 0 deletions src/ts/contentScripts/dispatcher/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,20 @@ const dispatchMap = new Map([

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

document.addEventListener('keydown', ev => {
const enter = 'Enter';
Copy link
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
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.

const isExitingTitle = (ev: KeyboardEvent) => {
return ev.target.parentElement instanceof HTMLHeadingElement;
Copy link
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..

}
const isNoBlockSelected = (ev: KeyboardEvent) => {
Copy link
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
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
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! ;)

return ev.target instanceof HTMLBodyElement;
}

if (ev.key === enter && (isExitingTitle(ev) || isNoBlockSelected(ev))) {
Roam.createBlockAtTop();
}
});

document.addEventListener('keyup', ev => {
if (ev.key === guard) replaceFuzzyDate();
});