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

Implement two-step caret movement at styled text boundaries #4269

Closed
Reinmar opened this issue Feb 9, 2018 · 10 comments · Fixed by ckeditor/ckeditor5-engine#1300
Closed
Assignees
Labels
package:engine type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Milestone

Comments

@Reinmar
Copy link
Member

Reinmar commented Feb 9, 2018

Background: see https://github.com/ckeditor/ckeditor5-link/issues/72.

  • We’ll implement only the “additional step on arrow key press” functionality.
    • You’re at a link boundary. The link is on.
      • NOTE: currently, the link is on if you’re at the end of the link. After this change it should be true also for the beginning of the link if the selection was moved there by using the left arrow key, but not if set there using mouse. This doesn't have to be a part of MVP – can be added later.
    • You press the right arrow key (if you’re at the end) and the link becomes disabled, but the caret doesn’t move.
    • You can press the left arrow key now to enable the link, but that won’t move the caret either.
    • This should work the same way on both ends of this style.
  • We need the same behaviour for highlight because removing highlight from collapsed selection removes it completely (the whole highlight), so you can’t easily stop typing with highlight. Hence, let’s implement it for all styles.
  • As the second step, after MVP, we should add a visual indicator that you’re typing with link. But this applies only to the link feature as it’s the most common feature which requires this indication.
  • Behaviour with multiple styles:
    • <b><i>x[]</b></i>x –> <b><i>x</b></i>[]x (gets out of all styles at once)
    • <b><i>x[]</b>x</i> –> <b><i>x</b>[]x</i>
  • There's an issue with the current proposal to listen on keydown (see https://github.com/ckeditor/ckeditor5-link/compare/hackathon-link-end) – selection attributes will be refreshed on next applyOperation which may fail when:
    • Collab changes come. Link may be restored.
    • You press Ctrl+B. Link may be restored.
  • IDEA: Implement DocumentSelection#overrideGravity(). The gravity should be overridden until the current user changes the selection. More or less. We can also allow defining how long the gravity is overridden by specifying your own callback or returning a function to be called to revert the gravity. Etc.
  • UNSURE: This becomes an engine feature. Where should the whole implementation land? How can features configure whether this works for a certain attribute? Should this be configurable? What would happen if you’d be at the boundary of two styles – one which works like this and the other which doesn’t?
@Reinmar
Copy link
Member Author

Reinmar commented Feb 9, 2018

Hence, let’s implement it for all styles.

I'm having a second thought about this. Without a visual indicator that something happened after your key press it might be confusing why the caret hasn't moved. In the classic or inline editors, the toolbar would act as an indicator, but in the balloon editor or Letters there will be no such thing.

Hence, let's maybe limit this behaviour to certain attributes only. Those which cause more troubles – links and perhaps highlight. The problem with the highlight feature is, though, that it's much harder to indicate that you left it. We'd need some tricks with outline perhaps.

cc @oleq @dkonopka

@pjasiun
Copy link

pjasiun commented Feb 9, 2018

Hence, let's maybe limit this behaviour to certain attributes only. Those which cause more troubles – links and perhaps highlight. The problem with the highlight feature is, though, that it's much harder to indicate that you left it. We'd need some tricks with outline perhaps.

I'm for it. For styles like bold or italic, it might be unexpected.

@scofalik
Copy link
Contributor

scofalik commented Feb 12, 2018

For bold or italic I'd go with Firefox'es solution, that is the selection style depends on from which side you reached the styled text.

EDIT: But then it might be weird that we have different solutions for different attributes/plugins.

@oleq
Copy link
Member

oleq commented Feb 13, 2018

TBH, I'd leave bold, italic, etc. as they are because one can always remove the selection attribute by pressing the toolbar button/keystroke.

I'd bring the new behavior for such features, which don't come with a possibility to clean up the selection without changing the content around (e.g. unlink removes the entire link).

@Reinmar
Copy link
Member Author

Reinmar commented Feb 13, 2018

Yeah, that's the current plan. In fact, we'll focus on the link feature first, since the highlight feature hasn't even landed yet.

@oskarwrobel
Copy link
Contributor

oskarwrobel commented Feb 13, 2018

@oskarwrobel
Copy link
Contributor

oskarwrobel commented Feb 15, 2018

I've created bindTwoStepCaretToAttribute( editor, emitter, attributeName ) helper/binding. My first thought was to put it to ui/bindings but this helper does not fit to UI stuff. I'm wondering about putting it to core/bindings. WDYT?

@Reinmar
Copy link
Member Author

Reinmar commented Feb 15, 2018

You could put it in the link pkg for now. It's going to be used only there for now. But if you already want to predict that highlight will use it, then we should find a place for it here in the engine, I think.

@oskarwrobel
Copy link
Contributor

engine/bindings ?

@pjasiun
Copy link

pjasiun commented Feb 15, 2018

engine/utils? Actually we shoud change binding folder in UI to utils or helpers, because binding folder name is legacy after the preview UI sructure.

Reinmar referenced this issue in ckeditor/ckeditor5-engine Feb 26, 2018
Feature: Introduced two-step caret movement mechanism. Closes #1289.
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-engine Oct 9, 2019
@mlewand mlewand added this to the iteration 14 milestone Oct 9, 2019
@mlewand mlewand added status:confirmed type:feature This issue reports a feature request (an idea for a new functionality or a missing option). package:engine labels Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:engine type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants