Skip to content

fix(LinkBubble): Don't open bubble at initialization and on remote changes #5924

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.

Sign up for GitHub

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

Merged
juliusknorr merged 7 commits into from
Jun 25, 2024
Merged

fix(LinkBubble): Don't open bubble at initialization and on remote changes #5924

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e784bd1
fix(LinkBubble): Don't open bubble when initializing the editor
mejo- Jun 18, 2024
89dc1d5
fix(LinkBubble): Don't open bubble on changes by other session members
mejo- Jun 19, 2024
16d33f4
test(cypress): Fix initial content undo test
mejo- Jun 19, 2024
96b9467
fix(LinkBubble): Fix race condition between hide and update
mejo- Jun 25, 2024
3550815
chore(LinkBubble): remove superfluous checks
mejo- Jun 25, 2024
a2f9f65
fix(LinkBubble): Hide link bubble when removing a link
mejo- Jun 25, 2024
bd1c2e2
test(cypress): Remove superfluous `cy.get()`
mejo- Jun 25, 2024
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
7 changes: 4 additions & 3 deletions cypress/e2e/initial.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,11 @@ describe('Test state loading of documents', function() {
.find('h2').should('contain', 'Hello world')

cy.getMenu().should('be.visible')
cy.getActionEntry('undo').should('be.visible').click()
cy.getActionEntry('undo').should('be.disabled')

cy.getContent()
.should('contain', 'Hello world')
.find('h2').should('contain', 'Hello world')
.type('New content')
cy.getActionEntry('undo').should('not.be.disabled')
})
})

Expand Down
3 changes: 0 additions & 3 deletions cypress/e2e/workspace.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,6 @@ describe('Workspace', function() {
cy.get('.file-picker [data-filename="test.md"]').click()
cy.get('.dialog__actions button.button-vue--vue-primary').click()

cy.getEditor()
.find('a')

cy.getContent()
.type('{leftArrow}')

Expand Down
6 changes: 5 additions & 1 deletion src/components/Link/LinkBubbleView.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,11 @@ export default {
},

removeLink() {
this.editor.chain().unsetLink().focus().run()
this.editor.chain()
.hideLinkBubble()
.unsetLink()
.focus()
.run()
this.stopEdit()
},
},
Expand Down
10 changes: 4 additions & 6 deletions src/plugins/LinkBubblePluginView.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,21 +92,19 @@ class LinkBubblePluginView {

update(view, oldState) {
const { active } = this.plugin.getState(view.state)
const { active: oldActive } = this.plugin.getState(oldState)
if (view.composing) {
return
}
if (active === oldActive) {
return
}
this.createTooltip()
if (active?.mark) {
this.updateTooltip(view, active)
setTimeout(() => {
this.updateTooltip(view, active)
}, 100)
} else {
this.removeEventListeners()
setTimeout(() => {
this.tippy?.hide()
}, 100)
this.removeEventListeners()
}
}

Expand Down
12 changes: 10 additions & 2 deletions src/plugins/links.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export function linkBubble(options) {
init: () => ({ active: null }),
apply: (tr, cur) => {
const meta = tr.getMeta(linkBubbleKey)
if (meta && meta.active !== cur.active) {
if (meta) {
return { ...cur, active: meta.active }
} else {
return cur
Expand All @@ -69,9 +69,17 @@ export function linkBubble(options) {
}),

appendTransaction: (transactions, oldState, state) => {
// Don't open bubble at editor initialisation
if (oldState?.doc.content.size === 2) {
Copy link
Collaborator

@max-nextcloud max-nextcloud Jun 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will also be in effect if you paste into an empty document. I can't think of any case where that's a problem. Just wanted to mention.

Copy link
Collaborator

@max-nextcloud max-nextcloud Jun 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uuhh... what if you insert a link via the link picker on an empty document. Actually would be nice if that behaved exactly as in a doc that already contains some characters. Not so important - but still a bit inconsistent.

Oh... doesn't the initial load happen through the ydoc? That would be caught by the other conditions later on - wouldn't it?

return
}

// Don't open bubble if neither selection nor doc changed
const sameSelection = oldState?.selection.eq(state.selection)
const sameDoc = oldState?.doc.eq(state.doc)
if (sameSelection && sameDoc) {
// Don't open bubble on changes by other session members
const noHistory = !transactions.some(tr => tr.meta.addToHistory)
if (sameSelection && (noHistory || sameDoc)) {
return
}
const active = activeLinkFromSelection(state)
Expand Down