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

Use a generic "widget with a selection handler" CSS class to make sure the handler is not cropped in all widgets #4598

Closed
oleq opened this issue Jan 17, 2019 · 2 comments · Fixed by ckeditor/ckeditor5-widget#67
Assignees
Labels
package:widget type:improvement This issue reports a possible enhancement of an existing feature.
Milestone

Comments

@oleq
Copy link
Member

oleq commented Jan 17, 2019

A follow–up of https://github.com/ckeditor/ckeditor5-block-quote/issues/28 where we brought some CSS to make sure that the selection handler of the table widget is not cropped when the table is a first-child of the block-quote or root editable (anything with overflow:hidden).

I think we should generalize the fix and instead of

.ck-editor__editable > .table:first-child,
.ck-editor__editable blockquote > .table:first-child {

we should refer to

.ck-editor__editable > .ck-widget.ck-widget_with-selection-handler:first-child,
.ck-editor__editable blockquote > .ck-widget.ck-widget_with-selection-handler:first-child {

when toWidget( ..., { hasSelectionHandler: true } ).

This will make the fix generic and work with all widgets we (and 3rd party developers) create in the future without constant updates to CSS.

In the future, if we develop different positions of the seleciton handler, we might need to make things dynamic, e.g. different styles (margin-top) when .ck-widget.ck-widget_with-selection-handler.ck-widget.ck-widget_selection-handler_top and (margin-botom) .ck-widget.ck-widget_with-selection-handler.ck-widget.ck-widget_selection-handler_bottom but I'm just thinking aloud.

cc @jodator @dkonopka

@oleq
Copy link
Member Author

oleq commented Jan 17, 2019

I just found out that we have the .ck-widget_selectable CSS class already added when hasSelectionHandler: true.

But it's totally wrong... all widgets are selectable (like the media widget, it has no handler but you can select it by simply clicking it). So we don't need to add another class, all we need to do is use this class but we need to rename it to something that makes sense. This class should indicate there's a selection handler, not that the widget it selectable (a very broad term).

@oleq oleq changed the title Add a class to widget with a selection handler to make sure the handler is not cropped in CSS Use a widget with a selection handler class to make sure the handler is not cropped in CSS Jan 17, 2019
@jodator
Copy link
Contributor

jodator commented Jan 17, 2019

Right now looks OK 👍

In the future, if we develop different positions of the seleciton handler, we might need to make things dynamic, e.g. different styles (

This looks promising so we shouldn't forget about it at least. Doing this right now without usages seems too early. But OTOH isn't a big change.

@oleq oleq self-assigned this Jan 17, 2019
@oleq oleq changed the title Use a widget with a selection handler class to make sure the handler is not cropped in CSS Use a generic "widget with a selection handler" CSS class to make sure the handler is not cropped in all widgets Jan 17, 2019
dkonopka referenced this issue in ckeditor/ckeditor5-table Jan 17, 2019
Internal: Renamed the .ck-widget_selectable class to .ck-widget_with-selection-handler for better semantics (see ckeditor/ckeditor5-widget#66).
dkonopka referenced this issue in ckeditor/ckeditor5-theme-lark Jan 17, 2019
Internal: Renamed the .ck-widget_selectable class to .ck-widget_with-selection-handler for better semantics. Made the selection handler crop fix introduced in #214 generic for all widgets (see ckeditor/ckeditor5-widget#66).
dkonopka referenced this issue in ckeditor/ckeditor5-widget Jan 17, 2019
Other: Renamed the .ck-widget_selectable class to .ck-widget_with-selection-handler for better semantics. Closes #66.

BREAKING CHANGE: The .ck-widget_selectable class has been renamed to .ck-widget_with-selection-handler for better semantics.
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-widget Oct 9, 2019
@mlewand mlewand added this to the iteration 22 milestone Oct 9, 2019
@mlewand mlewand added status:confirmed type:improvement This issue reports a possible enhancement of an existing feature. package:widget labels Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:widget type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants