Skip to content
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

Fake selection marker for collapsed and non-collapsed selections for link UI should be converted separately #8797

Merged
merged 3 commits into from
Jan 15, 2021

Conversation

niegowski
Copy link
Contributor

Suggested merge commit message (convention)

Other (link): Fake selection marker for collapsed and non-collapsed selections for link UI should be converted separately. Closes #8092.


Additional information

The problem with downcasting link-ui marker was that two separate converters were defined for the same marker:

  • markerToHighlight - for the non collapsed selection.
  • markerToElement - for the collapsed selection (but this converter also works correctly for non collapsed selection - it inserts elements at both ends of the marker range).

@scofalik
Copy link
Contributor

I reviewed this solution and I don't really like having two markers for the same thing. This is why I asked @niegowski to check alternative solution, which still has only one marker but the marker range is modified. It is similar to Radek's solution but it should be easier (less lines of code) as I proposed using position.getLastMatchingPosition() to move starting position to a proper place.

@niegowski
Copy link
Contributor Author

I reviewed this solution and I don't really like having two markers for the same thing. This is why I asked @niegowski to check alternative solution, which still has only one marker but the marker range is modified. It is similar to Radek's solution but it should be easier (less lines of code) as I proposed using position.getLastMatchingPosition() to move starting position to a proper place.

This solution is much simpler 🙂 I just pushed the fix.

@scofalik scofalik merged commit be55f91 into master Jan 15, 2021
@scofalik scofalik deleted the i/8092 branch January 15, 2021 21:07
@scofalik scofalik restored the i/8092 branch January 15, 2021 22:05
@arkflpc arkflpc deleted the i/8092 branch March 23, 2023 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Markers in empty elements can cause unexpected errors
2 participants