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

Introduce a way to reach unaccessible places (aka magicline and magicblock) - stage I #407

Closed
Reinmar opened this issue Feb 22, 2017 · 69 comments
Assignees
Labels
domain:accessibility This issue reports an accessibility problem. Epic package:widget squad:magic support:2 An issue reported by a commercially licensed client. type:bug This issue reports a buggy (incorrect) behavior. type:feature This issue reports a feature request (an idea for a new functionality or a missing option). type:improvement This issue reports a possible enhancement of an existing feature.

Comments

@Reinmar
Copy link
Member

Reinmar commented Feb 22, 2017

Problem:

image

In CKEditor 4 this problem was solved by the magicline plugin. We need some solution for CKEditor 5 as well.

The discussion started in relation to autoparagraphing. Interesting comments:


If you would like this feature to be implemented ASAP, please react with 👍 to this post.

@Reinmar Reinmar added status:confirmed type:improvement This issue reports a possible enhancement of an existing feature. type:feature This issue reports a feature request (an idea for a new functionality or a missing option). labels Feb 22, 2017
@Reinmar
Copy link
Member Author

Reinmar commented Nov 22, 2017

DUP reported in #685.

@Reinmar
Copy link
Member Author

Reinmar commented Jan 8, 2018

I promised to describe here some simpler workarounds for this problem. They are much easier to implement than Magicline and perhaps might even be a good start for us.

Additionally, I noticed two things when describing these solutions:

  • that with a bit more work those solutions would also solve the problem of broken Ctrl+A when widget is the first/last element of the content,
  • and that they don't conflict with a magicline-like solution that much (so they can be treated as addition to magicline, not a replacement).

Solution 1

  1. Listen to selection changes.
  2. Once discovering that an object type of element is selected (e.g. an image), insert a paragraph before/after it if it's at the very start/end of the content. Such insertion should happen in a transparent batch to not create an undo step.
  3. Remove these paragraphs if they are still empty after the selection is moved somewhere else.

Pros:

  • Quite simple to implement.
  • Well discoverable for the users.
  • Similar to how image captions work.

Cons:

  • There will be even more blinking (right now only captions appear when you click an image).
  • It would require additional work to filter these paragraphs out in getData() if it was called while an image was selected.
  • You need to know that you have to select the widget first to show these paragraphs.

Solution 2

Like in solution 1 but add these paragraphs only when someone:

  1. selects an image,
  2. presses an arrow key (e.g. down/right if the image is the last element of the content).

Pros:

  • Less intrusive than blinking paragraphs.
  • Quite simple to implement too.

Cons:

  • Less discoverable than Solution 1.

@Reinmar
Copy link
Member Author

Reinmar commented Apr 4, 2018

DUP reported in #935.

@Reinmar Reinmar added this to the backlog milestone Apr 5, 2018
@Reinmar
Copy link
Member Author

Reinmar commented Apr 8, 2018

@devgogo
Copy link

devgogo commented Apr 9, 2018

What's the good solution? 🔢

@syavash
Copy link

syavash commented May 4, 2018

@Reinmar Don't you think this should be categorized as a bug rather than an enhancement?

Most people type and then insert image at the end of the editor and as a result, cannot continue writing afterward.

Also, I think when a user clicks on image caption and hit enter, it should result in a new paragraph. Maybe, this can be a quick fix for images while the best solution is being decided on.

@Reinmar
Copy link
Member Author

Reinmar commented May 4, 2018

You may be right. For me, it's a missing feature because I know that it's how browsers work and we need to implement a feature which will allow reaching those places. For the users, this is purely a bug.

Still, it won't change much on how this ticket will be treated. We'd like to close it ASAP because we know that it makes for a bad UX, but we also know the scope of the problem (which is big).

@Reinmar Reinmar added the type:bug This issue reports a buggy (incorrect) behavior. label May 4, 2018
@Reinmar
Copy link
Member Author

Reinmar commented May 4, 2018

BTW, @oleq, you can start processing this issue in the back of your head ;)

@Reinmar
Copy link
Member Author

Reinmar commented May 4, 2018

cc @dkonopka too

@Reinmar Reinmar modified the milestones: backlog, next May 4, 2018
@syavash
Copy link

syavash commented May 4, 2018

@Reinmar I totally understand the scope and complications of this problem.

That's why I suggested that maybe initially we can just add a new paragraph when a user clicks on image's caption and hit enter. This won't be a solution when image caption is disabled, but I believe in any case that should be the behavior for captions. (Maybe, I'm wrong).

Thanks for the amazing job you guys are doing with ckeditor5.

@Reinmar
Copy link
Member Author

Reinmar commented May 4, 2018

That's a good question. In CKEditor 4 Enter opens widget dialogs (e.g. image properties). In CKEditor 5 we don't use dialogs and we used Tab for navigation so far. This leaves Enter unused.

My only worry with using only a keystroke is its discoverability. Still, it might be a good (and quick) start.

@balgev
Copy link

balgev commented May 14, 2018

is there any estimate for this issue?

@Reinmar
Copy link
Member Author

Reinmar commented Jun 14, 2018

We're about to merge a change which is going to allow inserting paragraphs before/after widgets by using Enter (to insert a paragraph after a widget) and Shift+Enter (to insert a paragraph before a widget). It works like this:

jun-14-2018 15-49-44

It doesn't solve this issue completely. It's sort of a workaround until we'll be able to introduce something with better discoverability (something that reacts to the mouse – like CKEditor 4's magicline).

Still, we'd like to hear your feedback.

PS. This change is going to be released around the next week.

Reinmar added a commit to ckeditor/ckeditor5-widget that referenced this issue Jun 14, 2018
Feature: Creating a paragraph next to the selected widget is possible using the (<kbd>Shift</kbd>+)<kbd>Enter</kbd> key (see ckeditor/ckeditor5#407).
@mjadobson
Copy link

mjadobson commented Jun 15, 2018

I think having a temporary fix is great, as this issue breaks the editor for users.

My concern would be that in other editors, pressing enter whilst selecting an image often deletes the image, as the intention is to replace it with a new line. (Albeit, most commonly when the image is inline, which isn't the case with ckeditor5)

Instead of waiting for magicline, gapcursor, etc., I think it would be worth mapping the suggested approach above to directional keys too, as mentioned in #407 (comment)

@Reinmar
Copy link
Member Author

Reinmar commented Jun 15, 2018

My concern would be that in other editors, pressing enter whilst selecting an image often deletes the image

I'm worried about this too. Although, I've realised recently when working on shift+enter that pressing enter on non-collapsed selections might be very uncommon. And that the expected result (inserting a line break) is quite surprising (at least for me) when experienced live.

Anyway, you may be right that with block images we're safer here. If we'd introduce the same for inline images it would be worse.

Instead of waiting for magicline, gapcursor, etc., I think it would be worth mapping the suggested approach above to directional keys too, as mentioned in #407 (comment)

I'm worried that the users would not know that they need to remove these paragraphs that they created this way (if they don't want to type in them). It's not typical for arrow keys to insert new elements.

Therefore, we considered this option but with an additional mechanism which would delete these paragraphs once you'd leave them without typing anything. Plus, which makes all this more complex, these paragraphs should not appear in the model (should be view-layer-only) to not cause some messy behaviours during real-time collaboration and when autosaving the content.

Reinmar added a commit that referenced this issue May 14, 2020
Feature (widget): Brought the feature allowing users to type in tight spots around block widgets where web browsers do not allow the caret to be placed (see #407). Closes #6740. Closes #6688. Closes #6689. Closes #6695.

Internal (horizontal-line): Updated the styling of .ck-horizontal-line from overflow: hidden to display: flow-root to allow the typing around UI inside the widgets (see #407, #6795).

Tests (image): Updated various image test to take the widget typing around feature into account (see #407).

Fix (media-mebed): The media widget conversion should not discard widget internals (drag or resize handlers, buttons to insert paragraphs, etc.) injected by other features when converting the URL (see #407).

Feature (theme-lark): Brought styles for the feature allowing users to type in tight spots around block widgets (see #407).

MINOR BREAKING CHANGE: The new --ck-color-focus-border-coordinates CSS custom property has been added and the existing --ck-color-focus-border property now uses it internally. If your integration overrides the latter, we recommend you update the former to avoid compatibility issues with various editor UI features.
@oleq
Copy link
Member

oleq commented May 14, 2020

For those who participated in this issue, I just wanted to let you know that the feature is being developed and the MVP already landed in master branch of the project 🎉 

There are still tons of improvements down the road (keyboard support in particular) so stay tuned.

If you're interested in remaining issues, you can find them under a dedicated label.

@zadam
Copy link

zadam commented May 15, 2020

Looks great!

Perhaps it's just me, but the arrow seemed to me at first sight unintuitive.

What I'm primarily trying to do is to put some text above the image, so I would expect the arrow to point upwards (in direction of my desired action). The fact that this action moves the image down (downward arrow) is just incidental.

@Reinmar
Copy link
Member Author

Reinmar commented May 15, 2020

What I'm primarily trying to do is to put some text above the image, so I would expect the arrow to point upwards (in direction of my desired action). The fact that this action moves the image down (downward arrow) is just incidental.

We considered arrows but they looked a lot more like "move this image one block up/down". The "enter key" arrow is not perfect too, but we couldn't think of a better icon so far.

@gkzsolt
Copy link

gkzsolt commented May 15, 2020

Does this fix allow to insert anything after an image ? We are waiting for this possibility for almost a year already, and can not upgrade to ckeditor 5 because it is unusable this way. There are tons of duplicate reports, and I see talking about "unaccessible places", "magic lines", a.s.o. many things I don't follow, but one use case is very simple:

You create a document, insert an image at the end and then cannot continue. You are doomed, there is no way to get past that image.

At least that was the situation last autumn/winter. So not before the image, and not a text inline with the image. A text below the image. Is that possible?

@neongreen
Copy link

You can select the image and press Enter (to insert after the image) or Shift-Enter (to insert before the image).

Does this fix allow to insert anything after an image ?

Still, I'm also curious about this question.

@oleq
Copy link
Member

oleq commented May 15, 2020

Does this fix allow to insert anything after an image ?

Yes. It inserts a paragraph. You can start typing or insert another image, table, etc.

@robido
Copy link

robido commented May 16, 2020

@oleq great work I can't wait to try it out!

@yann-yinn
Copy link

@oleq thank you so much, another huge ux improvement for ckeditor :D !

@otherblandart
Copy link

The tooltip seen on hover for this item doesn't appear to be localized yet, FYI
image

@oleq
Copy link
Member

oleq commented Jun 5, 2020

@otherblandart The feature is new and it will take some time for translations to arrive. Stay tuned or... if you want to contribute to the project, you can translate these labels by yourself. 

Thanks for your notice!

@Reinmar
Copy link
Member Author

Reinmar commented Jun 22, 2020

We extended the feature with another way to reach the space before/after a block – now the arrow keys can do that too.

We'll keep working on improving this concept, but I now consider the base case closed :) Finally 🎉

@Reinmar Reinmar closed this as completed Jun 22, 2020
@Reinmar
Copy link
Member Author

Reinmar commented Jun 22, 2020

You can see the potential followups in squad:magic .

@Reinmar Reinmar added the domain:accessibility This issue reports an accessibility problem. label Oct 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:accessibility This issue reports an accessibility problem. Epic package:widget squad:magic support:2 An issue reported by a commercially licensed client. type:bug This issue reports a buggy (incorrect) behavior. type:feature This issue reports a feature request (an idea for a new functionality or a missing option). type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
Development

No branches or pull requests