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

enh: improve selection experience #1264

Merged
merged 4 commits into from
Jul 19, 2023

Conversation

SystemChanger
Copy link
Contributor

@SystemChanger SystemChanger commented Jun 21, 2023

image

  1. Prevent 'floating' controls when their holder no longer exist
  2. Selection range matches component when it is selected (so we don't have to think about overlapping handlers)


const compContext = context[pluginName];
const container = compContext._container;
core.setRange(container, 0, container, 0);
Copy link
Owner

Choose a reason for hiding this comment

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

This code is a problem when entering text after selecting an image.
After the merge, this code will be removed.
Thank you for your contribution.!

Copy link
Contributor Author

@SystemChanger SystemChanger Jul 19, 2023

Choose a reason for hiding this comment

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

This change is crucial for my other PRs and is important for overall logic of selection - only one acceptable target at once (text, component or selection range). Such logic has a positive effect on many things - from removing flashing carriage on text to preventing overlapping handlers from different selection targets.
Are you talking about pressing text keys when there is a selected component? (that leads to text insertion to component figure area)
If I get you right, this problem was already there. (but may have different steps to reproduce the problem, e.g. pressing left/right arrow keys to position on component figure from the text node at first).
This code just makes this problem more explicit, and I also get rid of it in the further PRs.
Does it get fixed with next PR: https://github.com/JiHong88/suneditor/pull/1267/commits ?
I've done some enhancements and fixes in PRs, that altered the current behavior of keyboard interaction with components (images, videos) and explicit setting range to component area is really important.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I highly recommend including this code. Just a friendly reminder so you don't forget it when the time to work with PRs will come. @JiHong88

@JiHong88 JiHong88 merged commit e669387 into JiHong88:master Jul 19, 2023
@JiHong88 JiHong88 added this to the 2.45.2 milestone Mar 2, 2024
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.

2 participants