Several link handling fixes #5422
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
No longer hide the link bubble when editor looses focus. We just always show the link bubble as long as the selection stays within a link or until it's manually closed. This eliminates possible race-conditions and corner-case bugs. Signed-off-by: Jonas <jonas@freesources.org>
Prevents annoying race conditions when testing the LinkBubble in Cypress. Signed-off-by: Jonas <jonas@freesources.org>
Full URLs have the advantage that they also work when clicking them outside the Nextcloud context, e.g. in a local markdown editor. Their main disadvantage is that they cause trouble on Nextcloud instances with more than one hostname or after changing the hostname of a Nextcloud instance. The first use case (opening a file outside Nextcloud context) is more likely and less of a corner case than the second (changing hostname), so we decided to go for full URLs. Signed-off-by: Jonas <jonas@freesources.org>
juliusknorr
requested changes
Feb 28, 2024
| this.#updateDebounceTimer = window.setTimeout(() => { | ||
| this.updateFromSelection(view) | ||
| }, updateDelay) | ||
| debounce(() => this.updateFromSelection.bind(this)(view), updateDelay, { immediate: true }) |
There was a problem hiding this comment.
Does this debounce actually have an effect? As far as I know it would still call every time, unless you would call the returned function
There was a problem hiding this comment.
@max-nextcloud to the rescue 😬 I still struggle to understand the different scopes of JS functions 🙈
There was a problem hiding this comment.
Ah sorry, now I got what you mean. Indeed, debounce just returns a function that needs to be called. I thought it would be about the scope of this in updateFromSelection(). Anyways, should be fixed now
mejo-
force-pushed
the
fix/link_handling2
branch
from
4b7b23f to
814767f
Compare
mejo-
force-pushed
the
fix/link_handling2
branch
from
814767f to
4da1454
Compare
Run `updateFromSelection()` immediately on first call and only delay further runs. This fixes a race condition with `this.#hadUpdateFromClick` which gets set when updating from click and gets cleaned up after 200ms. Fixes an issue with the bubble disappearing immediately when clicking on a link after unfocussed browser and editor. Signed-off-by: Jonas <jonas@freesources.org>
Required to allow relative links (and those without origin) to other collectives pages to be resolved by link previews. Signed-off-by: Jonas <jonas@freesources.org>
Signed-off-by: Jonas <jonas@freesources.org>
Fixes: #5324 Signed-off-by: Jonas <jonas@freesources.org>
mejo-
force-pushed
the
fix/link_handling2
branch
from
4da1454 to
0dd249c
Compare
|
Cypress tests also green now 😊 |
|
Commit "fix(ActionInsertLink): Sync NcActionInput value property" should get backported to stable28, probably also to stable27. |
juliusknorr
approved these changes
Mar 1, 2024
|
/backport 0dd249c to stable28 |
|
/backport 0dd249c to stable27 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📝 Summary
🏁 Checklist
npm run lint/npm run stylelint/composer run cs:check)