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

Some toolbar items are active when editing html embed #8798

Closed
FilipTokarski opened this issue Jan 11, 2021 · 6 comments · Fixed by #8899
Closed

Some toolbar items are active when editing html embed #8798

FilipTokarski opened this issue Jan 11, 2021 · 6 comments · Fixed by #8899
Assignees
Labels
domain:ui/ux This issue reports a problem related to UI or UX. intro Good first ticket. package:html-embed package:table squad:core Issue to be handled by the Core team. type:bug This issue reports a buggy (incorrect) behavior.

Comments

@FilipTokarski
Copy link
Member

📝 Provide detailed reproduction steps (if any)

  1. Add html embed, click Edit source
  2. Add table / add media embed

✔️ Expected result

Table / media embed icons should be deactivated

❌ Actual result

Table / media embed is added after the html embed:

0_embed1


If you'd like to see this fixed sooner, add a 👍 reaction to this post.

@FilipTokarski FilipTokarski added type:bug This issue reports a buggy (incorrect) behavior. package:html-embed labels Jan 11, 2021
@mlewand mlewand added domain:ui/ux This issue reports a problem related to UI or UX. squad:core Issue to be handled by the Core team. labels Jan 18, 2021
@mlewand mlewand added this to the nice-to-have milestone Jan 18, 2021
@mlewand mlewand added the intro Good first ticket. label Jan 18, 2021
@oleq
Copy link
Member

oleq commented Jan 18, 2021

This could be an issue in the InsertTableCommand.

@oleq
Copy link
Member

oleq commented Jan 18, 2021

Looks like the media feature works the same.

@oleq
Copy link
Member

oleq commented Jan 18, 2021

It could be we decided to implement it this way because we didn't have the widget type around sub-feature and that was the only way, for instance, to insert content after an image if it was the last child of the editing root.

@oleq oleq modified the milestones: nice-to-have, backlog Jan 18, 2021
@magda-chrzescian magda-chrzescian modified the milestones: backlog, iteration 40 Jan 20, 2021
@magda-chrzescian magda-chrzescian self-assigned this Jan 20, 2021
@magda-chrzescian
Copy link
Contributor

Research

I have done some research regarding inserting things when something is selected and I would distinguish between two cases:

  • Inserting when the caret is visible and some content can be edited, but it is not the root of the editor (HTML snippets, captions, maybe also code blocks) - In this case, the user may expect the content to be inserted right where the caret is placed at the particular moment.
  • Inserting things when a widget is selected - This one is not the scope of this issue but may be worth considering if inserting other widgets should be allowed in this case.

So, regarding the first case:

Word

Set caret in the text box -> Insert a table -> The table is being inserted inside the text box
Set caret in the text box -> Insert an image -> The image is being inserted inside the text box
When the caret is inside the text box, all of the buttons for inserting things that are not allowed are disabled.

Gogle Docs

There is nothing to test on.

TinyMCE

The code sample can not be edited in the scope of the editor (it is edited in the pop-up).

ProseMirror

The embedded code editor doesn’t allow inserting things. Only the horizontal line is allowed and it lands before the code block, but it feels more like a bug.


Conclusion

I think we should just disable the commands for inserting things that are not allowed to be in the HTML snippet.

@magda-chrzescian
Copy link
Contributor

Since the html-embed widget is the rawElement, it can not be determined if the content of the widget is being edited at the given moment. Therefore, the decision about enabling/disabling the commands has to be made based on the currently selected element.

We decided to disable the insertTable command if any object is selected and to disable the instertMediaEmbed command if any object other than media is selected.

oleq added a commit that referenced this issue Jan 26, 2021
…html

Fix (table): The `insertTable` command should be disabled if any object is selected. Closes #8798.

Fix (media-embed): The `insertMediaEmbed` command should be disabled if any non-media object is selected (see #8798).

Other (widget): The `checkSelectionOnObject` function should be exported by the `@ckeditor/ckeditor5-widget/src/utils package` (see #8798).

Tests (html-embed): The manual test should show the insert table and insert media buttons and describe their correct state (see #8798).
@Rae-Lin
Copy link

Rae-Lin commented Aug 17, 2021

Hello @oleq @magda-chrzescian,
I'm new at this and it's my first time using ckeditor.
I used CKEditor 5 online builder and imported in my angular project.

Would you please tell me step by step what should I do if I also want to disable command in embed html?
Many thanks. <3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:ui/ux This issue reports a problem related to UI or UX. intro Good first ticket. package:html-embed package:table squad:core Issue to be handled by the Core team. type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants