Fix/hide bubble when in menu#5529

Merged

max-nextcloud merged 23 commits intomainfrom

fix/hide-bubble-when-in-menu

Mar 20, 2024

Collaborator

max-nextcloud commented Mar 19, 2024

📝 Summary

See: Link preview follow up #5442
Move the event handling into the LinkBubble plugin and expose a command to hide the link bubble.
Make use of that command to hide the link bubble when opening the preview menu.

🏁 Checklist

Code is properly formatted (npm run lint / npm run stylelint / composer run cs:check)
Sign-off message is added to all commits
[Tests] lower level implementation of the hide command is covered with unit test. Tested the ui by hand in various scenarios (write / read only, firefox / chromium)
Documentation is not required

max-nextcloud force-pushed the fix/hide-bubble-when-in-menu branch 2 times, most recently from 43aedac to 6fe12c9 Compare

March 19, 2024 12:41

max-nextcloud requested review from juliusknorr and mejo-

March 19, 2024 12:43

max-nextcloud added 3. to review 29 feedback labels

max-nextcloud self-assigned this

mejo- approved these changes

View reviewed changes

Member

mejo- left a comment

Really nice work 🤩

I went through all the commits and think I followed most of the code changes. Lots of nice refactoring and cleanups in there!

I also did some manual testing around the link bubble and everything seems work reliable there. Just one nitpicking comment.

src/plugins/LinkBubblePluginView.js Outdated Show resolved Hide resolved

Member

mejo- commented Mar 19, 2024

Not sure whether the failing Cypress test "Link website with selection" from Links.spec.js is related though.

Collaborator Author

max-nextcloud commented Mar 20, 2024

Not sure whether the failing Cypress test "Link website with selection" from Links.spec.js is related though.

It probably is. I think the link happens in the a tag but behind the text. It then triggers both the click handler and the selection update. The selection update will remove the link bubble again as the cursor is behind the link not inside it.

I'm tending to make the selection handling more inclusive - that is also show the link bubble when the cursor is in front of the link or behind the link as these cursor positions can be reached by clicking the link.

max-nextcloud force-pushed the fix/hide-bubble-when-in-menu branch from d068db7 to dcb7dc6 Compare

March 20, 2024 07:27

max-nextcloud added 19 commits

March 20, 2024 10:52


          refactor(links): move linkBubble plugin to plugins/links

710b350

Signed-off-by: Max <max@nextcloud.com>


          refactor(links): pass editor via options to LinkBubblePluginView

e6049b0

Signed-off-by: Max <max@nextcloud.com>


          refactor(links): untangle LinkBubblePluginView from editor

d871277

It is a prosemirror plugin view. No need for it to know tiptap data structures

Signed-off-by: Max <max@nextcloud.com>


          refactor(links): use prosemirror tr and state to track clicks

e4340f8

Signed-off-by: Max <max@nextcloud.com>


          refactor(links): move click handling into link bubble plugin

90b99be

Signed-off-by: Max <max@nextcloud.com>


          refactor(links): only hand link mark to updateTooltip

356509b

Signed-off-by: Max <max@nextcloud.com>


          refactor(links): introduce setActiveLink function

e30c78c

Also rename `clicked` state to `active`.
We will also use it for activating via keypress

Signed-off-by: Max <max@nextcloud.com>


          fix(links): also update if active was unset

d956fdd

Signed-off-by: Max <max@nextcloud.com>


          refactor(links): operate on state with linkNodeFromSelection

e02c5ad

In appendTransation callbacks there is no view available.

Signed-off-by: Max <max@nextcloud.com>


          refactor(links): move linkNodeFromSelection into helper

c1e4bd8

Signed-off-by: Max <max@nextcloud.com>


          refactor(links): handle selection changes in plugin

96097aa

Signed-off-by: Max <max@nextcloud.com>


          refactor(links): use hideLinkBubble command for esc

4a3165b

Signed-off-by: Max <max@nextcloud.com>


          fix(links): simplify updateTooltip and handle active null

ed923f4

Signed-off-by: Max <max@nextcloud.com>


          fix(links): remove special handling for clicked and focus

4bc7586

Signed-off-by: Max <max@nextcloud.com>


          fix(links): handle esc in DOMEvents rather than prop

7fe10f9

The `handleKeyDown` prop does not work in read only mode.

Signed-off-by: Max <max@nextcloud.com>


          fix(links) handle linkNodeFromSelection returning false

d79b67c

Signed-off-by: Max <max@nextcloud.com>


          enh(links): close link bubble when opening preview toggle

4f5277e

Signed-off-by: Max <max@nextcloud.com>


          fix(lint): add missing jsdoc comments to new files

d582f38

Signed-off-by: Max <max@nextcloud.com>


          docs(preview): improve jsdocs for markdownit preview plugin

322a752

Signed-off-by: Max <max@nextcloud.com>

max-nextcloud added 4 commits

March 20, 2024 10:52


          refactor(links): only get mark from selection

8e78cb7

Signed-off-by: Max <max@nextcloud.com>


          refactor(links): get active link from selection

385b246

Signed-off-by: Max <max@nextcloud.com>


          fix(links): show bubble with cursor on the end of the link

c2fd1e5

Signed-off-by: Max <max@nextcloud.com>


          tests(links): ignore link bubble when reading link text

fb0a8ce

The link bubble now also gets rendered
when the cursor is behind the link.

`cy.get` will always start at the root of the doc.
So it also found the link inside the link bubble.
Use `.find` on the content instead as the link bubble is outside of that.

Signed-off-by: Max <max@nextcloud.com>

max-nextcloud force-pushed the fix/hide-bubble-when-in-menu branch from dcb7dc6 to fb0a8ce Compare

March 20, 2024 09:53

max-nextcloud enabled auto-merge

March 20, 2024 09:53

max-nextcloud merged commit 5dfe6a6 into main

max-nextcloud deleted the fix/hide-bubble-when-in-menu branch

March 20, 2024 10:04

Altahrim mentioned this pull request

29.0.0 beta 5 nextcloud/server#44361

Merged

mejo- mentioned this pull request

Link bubble: click outside no longer closes the bubble #5539

Closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review 29 feedback