Vim Mode 2 - Various helpers I use to implement Vim Mode #1

tntmarket · 2020-06-23T08:30:26Z

This is part of a larger change to support vim style navigation (roam-unofficial#63)

This includes utilities to work with lists and objects and wrappers around browser APIs.

Stvad

a lot of these functions are amenable to and would benefit from testing :)

Stvad · 2020-06-24T03:36:25Z

src/ts/core/common/assert.ts

@@ -0,0 +1,7 @@
+


Would you prefer I use foo!.bar instead? (I recently learned that Typescript has this feature).

The explicit asserts sometimes save me time when debugging, but I recognize they clutter the code.

this was just about newline 😛. I believe !. does not change the runtime behavior so your assert is actually different, I'm totally fine to keep it :)

tntmarket · 2020-06-25T04:20:26Z

src/ts/core/common/keyboard.ts

@@ -9,7 +9,7 @@ export const Keyboard = {
    BASE_DELAY: 20,

    async simulateKey(code: number, delayOverride: number = 0, opts?: KeyboardEventInit) {
-        ;['keydown', 'keyup'].forEach(eventType =>
+        ['keydown', 'keyup'].forEach(eventType =>


src/ts/scripts/dom.ts

@@ -0,0 +1,11 @@
+export const injectStyle = (css: string, tagId: string) => {


src/ts/scripts/mutation-observer.ts

@@ -0,0 +1,42 @@
+import {assumeExists} from 'SRC/core/common/assert'


Stvad · 2020-06-24T04:47:36Z

src/ts/scripts/mutation-observer.ts

+    return () => waitForLoad.disconnect()
+}
+
+export const waitForSelectorToExist = (selector: string, observeInside = document.body) => {


in a parallel PR I recommended using https://github.com/uzairfarooq/arrive - I'm wondering if we should use it here ?

I'll try it out

I tried arrive, and I'm not sure it's worth it.

Observing right panel toggles via arrive looks like this:

import Arrive from 'arrive' // Stick the variable somewhere, so arrive can apply it's monkey patches // Otherwise, webpack will shake it away Arrive export const RoamEvent = { onRightPanelToggle(): { document.querySelector(Selectors.rightPanel).arrive(Selectors.sidebarContent, () => { console.log('right panel opened') }) document.querySelector(Selectors.rightPanel).leave(Selectors.sidebarContent, () => { console.log('right panel closed') }) }, }

Observing page addition/removal from the sidebar looks like this:

document.querySelector(Selectors.sidebarContent).arrive('div', () => { console.log('something might've been added to right panel') }) document.querySelector(Selectors.sidebarContent).leave('div', () => { console.log("something might've been removed from right panel") })

arrive observes the whole subtree, so div arrivals actually trigger many times whenever anything in the right panel changes. I'm paranoid that this may impact performance, or have unintended side effects if someone is just editing blocks inside the sidebar

arrive also doesn't have typescript definitions

Stvad · 2020-06-24T05:06:51Z

src/ts/core/roam/roam-block.ts

+    return navigator.clipboard.writeText(`{{embed: ((${getBlockUid(blockId)}))}}`)
+}
+
+export const copyBlockReference = (blockId: string | undefined) => {


name parameter something like htmlBlockId to distinguish from roam block?

Stvad · 2020-06-24T05:07:51Z

src/ts/core/roam/roam.ts

+        const foldButton = assumeExists(
+            assumeExists(block.parentElement).querySelector(Selectors.foldButton)
+        );
+        await Mouse.hover(foldButton as HTMLElement);


move type cast to variable declaration

Stvad · 2020-06-24T05:11:13Z

src/ts/core/common/object.ts

@@ -0,0 +1,37 @@
+export const invertObject = <K, V>(obj: {


another thing we can get from lodash

Stvad · 2020-06-24T05:13:02Z

src/ts/core/common/object.ts

+    [key: string]: V,
+}
+
+export const mapObjectValues = <X, Y>(obj: ObjectMap<X>, mapFn: (x: X) => Y): ObjectMap<Y> => {


another thing we can get from lodash. 's probably enough justification to take a dependency on it (or similar lib?) by this point

I'll use lodash. I think webpack tree-shakes the methods we don't use, so it should be cheap

Stvad · 2020-06-24T05:17:06Z

src/ts/scripts/mutation-observer.ts

+        handleChange()
+    })
+
+    waitForLoad.observe(observeInside || assumeExists(document.querySelector(selector)), {


should we have separate functions for observing the element and selector?

I'll decouple picking which element to observe from actually observing it

…am helpers

…s.ts

…her than introducing a seperate method for that

…ason :(

Stvad

looks good to me now :)

Stvad · 2020-06-27T02:43:19Z

src/ts/core/roam/roam.ts

@@ -19,7 +22,7 @@ export const Roam = {

    getRoamBlockInput(): HTMLTextAreaElement | null {
        const element = getActiveEditElement()
-        if (element.tagName.toLocaleLowerCase() !== 'textarea') {
+        if (!element || element.tagName.toLocaleLowerCase() !== 'textarea') {


nit: use ?.

Stvad · 2020-06-27T02:52:04Z

src/ts/core/common/mutation-observer.ts

+
+const observeElement = (
+    observeInside: HTMLElement,
+    handleChange: () => void,


wonder if this should accept the changed element.

Stvad · 2020-06-27T02:57:54Z

src/ts/core/common/keycodes.ts

+import {invertObject} from 'SRC/core/common/object'
+
+// 2nd column represents shifted keys
+export const KEY_TO_CODE: {[key: string]: number} = {


yeah, the amount of workarounds we need in #2 is very sad =\

…I still use the `observeInside` option of `waitForSelectorToExist` later

Stvad · 2020-06-28T00:06:37Z

npm run test is failing now :(

tntmarket · 2020-06-28T00:08:00Z

Oh dang, I'll investigate

tntmarket · 2020-06-28T00:10:25Z

Hmm I think I need to update the import paths in the existing tests, after allowing absolute imports in jest

tntmarket · 2020-06-28T00:14:43Z

Opened a new PR with the fix: roam-unofficial#94

Stvad · 2020-06-28T00:16:13Z

thanks!

tntmarket mentioned this pull request Jun 23, 2020

Many vim-like shortcuts for navigating and manipulating blocks roam-unofficial/roam-toolkit#63

Closed

Stvad reviewed Jun 24, 2020

View reviewed changes

Allow absolute imports

4a765f0

tntmarket force-pushed the vim_mode_1_allow_absolute_imports branch from 5d12c6e to 4a765f0 Compare June 25, 2020 03:47

tntmarket force-pushed the vim_mode_2_various_utilities_to_that_are_used_by_vim_mode branch from 8ce0c2b to c695d4c Compare June 25, 2020 04:58

tntmarket added 4 commits June 24, 2020 22:26

Various utilities for browser APIs, generic data manipulation, and Ro…

19bb111

…am helpers

Allow absolute imports in jest tests

d692ced

Unit test relativeItem

1d2b7e6

Unit test repeatAsync

53d9f01

tntmarket force-pushed the vim_mode_2_various_utilities_to_that_are_used_by_vim_mode branch 2 times, most recently from 8b9e383 to 988e7df Compare June 25, 2020 05:29

tntmarket added 8 commits June 24, 2020 22:40

have isEditing return false if no activeElement exists

6f36c2c

Run prettier

e5f4c92

Move prettier ignore comment

eb79ac8

Add TODOs to switch to lodash in later commits

11c8a64

Clarify naming of blockId, rename roam-block => block

15e239e

Reorganize to remove redundant typecast

15a4ed5

Use injectStyle for custom css feature, and move it to core/common/cs…

4f11583

…s.ts

Move mutation-observer to core/common

6e43e9e

tntmarket force-pushed the vim_mode_2_various_utilities_to_that_are_used_by_vim_mode branch from 988e7df to 6e43e9e Compare June 25, 2020 05:41

tntmarket added 4 commits June 24, 2020 23:15

Actually, don't need the key codes for shifted keys

44f79e1

Remove unnecessary newline

b35fe5e

Make getActiveEditElement return null when no element is editing, rat…

8578dfe

…her than introducing a seperate method for that

Just update npm lockfile. npm dedupe breaks npm install for some re…

309d6c1

…ason :(

tntmarket force-pushed the vim_mode_2_various_utilities_to_that_are_used_by_vim_mode branch from 8d89a2a to 309d6c1 Compare June 26, 2020 20:28

Reorganize mutation-observer methods to have less cluttered signatures

e82289f

Stvad approved these changes Jun 27, 2020

View reviewed changes

tntmarket changed the base branch from vim_mode_1_allow_absolute_imports to master June 27, 2020 22:33

tntmarket added 2 commits June 27, 2020 15:38

Use .? instead of null check

c16253a

Pass back the changed element when observing it. Also, I forgot that …

a06d6b5

…I still use the `observeInside` option of `waitForSelectorToExist` later

tntmarket merged commit f2b9bd0 into master Jun 27, 2020

tntmarket mentioned this pull request Jun 27, 2020

Various utilities for browser APIs, generic data manipulation roam-unofficial/roam-toolkit#93

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Vim Mode 2 - Various helpers I use to implement Vim Mode #1

Vim Mode 2 - Various helpers I use to implement Vim Mode #1

tntmarket commented Jun 23, 2020

Stvad left a comment

Stvad Jun 24, 2020

tntmarket Jun 25, 2020 •

edited

Loading

Stvad Jun 26, 2020

This comment was marked as resolved.

tntmarket Jun 25, 2020

This comment was marked as resolved.

This comment was marked as resolved.

Stvad Jun 24, 2020

tntmarket Jun 25, 2020

tntmarket Jun 26, 2020 •

edited

Loading

Stvad Jun 27, 2020

Stvad Jun 24, 2020

Stvad Jun 24, 2020

Stvad Jun 24, 2020

Stvad Jun 24, 2020

tntmarket Jun 24, 2020

Stvad Jun 24, 2020

tntmarket Jun 26, 2020

Stvad left a comment

Stvad Jun 27, 2020

Stvad Jun 27, 2020

Stvad Jun 27, 2020

Stvad commented Jun 28, 2020

tntmarket commented Jun 28, 2020

tntmarket commented Jun 28, 2020

tntmarket commented Jun 28, 2020

Stvad commented Jun 28, 2020

		@@ -0,0 +1,11 @@
		export const injectStyle = (css: string, tagId: string) => {

		@@ -0,0 +1,42 @@
		import {assumeExists} from 'SRC/core/common/assert'

Vim Mode 2 - Various helpers I use to implement Vim Mode #1

Vim Mode 2 - Various helpers I use to implement Vim Mode #1

Conversation

tntmarket commented Jun 23, 2020

Stvad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tntmarket Jun 25, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

Choose a reason for hiding this comment

This comment was marked as resolved.

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tntmarket Jun 26, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Stvad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Stvad commented Jun 28, 2020

tntmarket commented Jun 28, 2020

tntmarket commented Jun 28, 2020

tntmarket commented Jun 28, 2020

Stvad commented Jun 28, 2020

tntmarket Jun 25, 2020 •

edited

Loading

tntmarket Jun 26, 2020 •

edited

Loading