-
Notifications
You must be signed in to change notification settings - Fork 94
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
fix(LinkBubble): Don't open bubble at initialization and on remote changes #5924
Conversation
/backport to stable29 |
8212160
to
8b413f9
Compare
Fixes: #5855 Fixes: nextcloud/collectives#1296 Signed-off-by: Jonas <[email protected]>
a837f07
to
337b9dc
Compare
Signed-off-by: Jonas <[email protected]>
Undo button should be disabled after initial content got loaded and only get enabled after typing something. Signed-off-by: Jonas <[email protected]>
Hide was called with a timeout of 100ms, while update was not. This could lead to a race condition and flaky link bubble behaviour. Signed-off-by: Jonas <[email protected]>
337b9dc
to
a6e80a4
Compare
`active` object from prosemirror state and oldState are never similar anyway. Signed-off-by: Jonas <[email protected]>
Signed-off-by: Jonas <[email protected]>
a6e80a4
to
a2f9f65
Compare
Should make the workspace link bubble test less flaky. Signed-off-by: Jonas <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
@@ -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) { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
Fixes: #5855
Fixes: nextcloud/collectives#1296
🏁 Checklist
npm run lint
/npm run stylelint
/composer run cs:check
)