roam-toolkit icon indicating copy to clipboard operation
roam-toolkit copied to clipboard

Highlighting hint `w` shortcuts not work in page embed state

Open JimmyLv opened this issue 2 years ago • 9 comments

https://user-images.githubusercontent.com/4997466/165499485-d2505b28-5137-4576-adbf-1b0611439cbb.mp4

for example:

#Tag {{[[embed]]: [[Test Hello World!]]}}

In this scenario, w is not in effect and cannot open the embed [[Test Hello World!]] page title

JimmyLv avatar Apr 27 '22 10:04 JimmyLv

is it related here? https://github.com/roam-unofficial/roam-toolkit/blob/770bc52395/src/ts/core/common/mouse.ts#L19

JimmyLv avatar Apr 27 '22 13:04 JimmyLv

can confirm/reproduce. it seems to be related to how the html for that case is structured (span inside a), so the hint gets attached to the wrong element

Stvad avatar Apr 28 '22 00:04 Stvad

@Stvad wow, great! Is it easy to fix? I'm also more than happy to help if needed! I would love this fix, 😁

JimmyLv avatar Apr 28 '22 08:04 JimmyLv

Not sure, I think the fix would be to see if the elements found have sub-element that are links here https://github.com/roam-unofficial/roam-toolkit/blob/770bc52395d77990bb0fa80887f41df70d3dba62/src/ts/core/features/vim-mode/hint-view.ts#L51 And attach the class to those instead, but I’m not sure if that’d produce the desired behavior reliably. Contributions are welcome!

Stvad avatar Apr 28 '22 19:04 Stvad

ok, but how can I access and click these sub-elements (span inside a)? do you mean the click() method would dispatch the event by the attached classes here?

JimmyLv avatar Apr 30 '22 08:04 JimmyLv

Google Chrome-DevTools - roamresearch com-001955-20220430 Does that mean these classes roam-toolkit--hint roam-toolkit--hint1 should be attached to the span element?

JimmyLv avatar Apr 30 '22 08:04 JimmyLv

Hey @Stvad , I just created the PR and deployed it to vercel, but it doesn't look like there is a roam/js version? https://roam-toolkit.vercel.app/no_preview_spatial.js

JimmyLv avatar Apr 30 '22 08:04 JimmyLv

Apologies, I missed this! roam/js version is built off the separate branch atm (roam-js see #164 )

Stvad avatar May 13 '22 21:05 Stvad

wow, thanks a lot! I just deployed and my PR seems to work well. https://roam-toolkit.vercel.app/entry.js

JimmyLv avatar May 14 '22 02:05 JimmyLv