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.

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

Jump to bottom

InputCommand should accept range instead of position as a parameter. #3082

Closed
f1ames opened this issue Mar 13, 2017 · 0 comments · Fixed by ckeditor/ckeditor5-typing#88
Closed

InputCommand should accept range instead of position as a parameter. #3082

f1ames opened this issue Mar 13, 2017 · 0 comments · Fixed by ckeditor/ckeditor5-typing#88
Assignees
@f1ames
Labels
package:typing type:bug This issue reports a buggy (incorrect) behavior.
Milestone
iteration 9

Comments

@f1ames
Copy link
Contributor

f1ames commented Mar 13, 2017

While InputCommand uses position to restore selection (https://github.com/ckeditor/ckeditor5-typing/blob/master/src/inputcommand.js#L67) it works in most cases due to the fact that selection is collapsed after text insertion.

The case where selection is not collapsed is MacOS accent panel. While navigating through the accents, native selection is non-collapsed containing newly inserted accent. While the input command uses position, it collapses selection in a view/model which causes additional rendering step (because selection in DOM is not collapsed). Using Range instead of Position will prevent this situation.

It is important due to the fact that rerendering selection during the composition breaks it in Safari.
@f1ames f1ames self-assigned this Mar 13, 2017
@f1ames f1ames changed the title InputCommand should accept range instead of position as an parameter. InputCommand should accept range instead of position as a parameter. Mar 13, 2017
f1ames referenced this issue in ckeditor/ckeditor5-typing Mar 13, 2017
@f1ames
Merge pull request #88 from ckeditor/t/86
0766407 
Fix: `InputCommand` now accepts `Range` instead of `Position` as a parameter. Closes #86. Closes #54.
BREAKING CHANGE: `InputCommand` `options.resultPosition` was replaced with `options.resultRange`.
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-typing Oct 9, 2019
@mlewand mlewand added this to the iteration 9 milestone Oct 9, 2019
@mlewand mlewand added status:confirmed type:bug This issue reports a buggy (incorrect) behavior. package:typing labels Oct 9, 2019
@f1ames f1ames mentioned this issue Oct 11, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@f1ames f1ames

Labels
package:typing type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Milestone
iteration 9
Development

Successfully merging a pull request may close this issue.

T/86 InputCommand should accept range instead of position as a parameter. ckeditor/ckeditor5-typing
2 participants
@f1ames @mlewand