Opened 5 years ago

Last modified 5 years ago

#12019 confirmed Bug

Styles' states and values when block widget is selected

Reported by: Piotrek Koszuliński Owned by: Piotrek Koszuliński
Priority: Normal Milestone:
Component: UI : Widgets Version: 4.3 Beta
Keywords: Cc:

Description (last modified by Piotrek Koszuliński)

  1. Open http://ckeditor.dev/plugins/widget/dev/nestedwidgets.html
  2. Focus one of block widgets.
  3. See format and styles drop-downs, basic styles, etc. All are enabled and many times values are incorrect - e.g. format is set to "normal div" for image2 inside nested editable (additionally, this is not consistent between framed and inline editors).

All these behaviours come from a decision we made while working on widget system. We decided that styles will be applicable to block widgets, because they may contain nested editables in which these styles may be applicable, even if they are not applicable to the widget itself (and besides widget styles they never are). This reasoning definitely makes sense - if I have 3 nested editables inside a widget and I want to make all text inside them bold I can focus widget and apply bold. Alternatively, I would have to apply bold three times. To make all this work we needed to modify style system so it traverse non-editable fragments of content, finds nested editables, retrieves their filter instances and asks them whether styles is allowed inside. This part works great.

Unfortunately, all the rest is a mess. Problems are caused by inability to correctly set style's command state and value. See the following scenarios.

  1. Focus captioned image2 and apply bold. Bold is applied in the caption. Now, try to remove the bold. You can't, because bold button is still off after applying bold. Why is it off? Because <figure> is not inside a <strong>. Are we able to set the state to on? I don't think so - this would mean traversing focused widget's DOM, finding nested editables and then what? Enabling style if it's applied to entire content of all nested editables? Or at the beginning of any editable? In any case this would be a performance killer, because we not only need to find editables, but also first editable place inside them (text node, image, space before <br>, widget, etc).
  2. Imagine the above scenario with captioned image2 nested in simple box. The algorithm will need to go inside that image2 when simple box is selected.
  3. What is the expected format drop-down's value when simplebox is focused and it its content there are headings and paragraphs? Simplebox has a header in which format should be disabled. Then it has content, which may start with a normal block (paragraph, heading) or for example a captioned image. Image does not allow changing format (so should drop-down be disabled?), but after image there may be paragraphs, so maybe format should be enabled (but then what with the value?).
  4. Imagine all these scenarios with widgets containing multiple nested editables of different types. User will never intuitively know what will happen and what's just happened.

And I'm sure that there are move lovely cases like these.

Therefore, I don't see other option than disabling styles when block widget is focused. Only widgets styles should be active.

Note that this won't change the behaviour of applying style to a selection containing some content outside a widget and a widget (e.g. bolding after doing CTRL+A on text with captioned images). Style will still be applied to all places in which it's allowed. There's a substantial difference between these cases - the start of selection. When selection does not contain only a widget, it starts in an editable place, so we can check style's state in that place.

Change History (11)

comment:1 Changed 5 years ago by Piotrek Koszuliński

Status: newconfirmed

comment:2 Changed 5 years ago by Piotrek Koszuliński

Description: modified (diff)

comment:3 Changed 5 years ago by Piotrek Koszuliński

Owner: set to Piotrek Koszuliński
Status: confirmedassigned

comment:4 Changed 5 years ago by Piotrek Koszuliński

Description: modified (diff)
Summary: [Nested widgets] Format value is "Normal (div)" when nested block widget is selectedStyles yield to be applicable when block widget is selected

Again this turned out to be a problem with widgets in general, rather than with nested widgets. Pushed tests in branch:t/12019 that proves it.

comment:5 Changed 5 years ago by Piotrek Koszuliński

Status: assignedpending
Summary: Styles yield to be applicable when block widget is selectedStyles' states and values when block widget is selected

comment:6 Changed 5 years ago by Piotrek Koszuliński

Replying to an idea presented to me outside the dev channel:

We may think about doing a lookup, for inline styles only, checking if it is applied in the beginning of the very first editable spot inside a widget. We need to check if it is both possible and if it could perform well.

I think that is as hard as a full support. We would still need to find the first nested editable, the first editable place inside it, perhaps repeat that if a widget was discovered at this position, etc. Additionally, the first nested editable may have that style disallowed, so then we should check a next nested editable, because that style could be applied in it.

comment:7 Changed 5 years ago by Anna Tomanek

Wiktor asked me to add a comment to this ticket as I apparently tried to do a strange thing today, selecting a widget and clicking the Link button on it. The results are inconsistent, for example:

  • For image2, codesnippet & simplebox the Link button is active, but nothing happens after you add a link by closing the Link dialog window with OK (no sign in source).
  • For placeholder and mathjax links are actually added and when you double-click the linked widget, two dialog windows open, one over another - for link and for the widget.

So perhaps it's not just the question of styles, but allowed features (like links) as well.

comment:8 Changed 5 years ago by Piotrek Koszuliński

If you use link on image2 the image is linked - this works perfectly. But it's exactly the reason why we should not apply styles inside nested editables. Thank you for it :D. You were expecting that the caption will be linked, when... the image was linked. If you weren't expecting that caption will be linked you would have bigger chance to notice that an images' border appeared. Link appears in the retrieved data too.

And yes - for all other block widgets link should be disabled. It is now correctly applied in simple box'es content, but this is the same case as other styles (link is a special kind of style in fact).

For placeholder and mathjax links are actually added and when you double-click the linked widget, two dialog windows open, one over another - for link and for the widget.

This must be a recent regression. Reported: #12140.

comment:9 Changed 5 years ago by Anna Tomanek

If you use link on image2 the image is linked - this works perfectly. But it's exactly the reason why we should not apply styles inside nested editables. Thank you for it :D. You were expecting that the caption will be linked, when... the image was linked. If you weren't expecting that caption will be linked you would have bigger chance to notice that an images' border appeared. Link appears in the retrieved data too.

I wasn't really expecting anything, to be honest; I was just surprised that some buttons were enabled when a widget was selected, so I just tried to see what happens when I use them :) To sum up, it's probably not any sort of a valid scenario that a user would follow, I just find it confusing that a feature (Link in this case) seems to be available while in fact it does not work as expected.

And yes - for all other block widgets link should be disabled. It is now correctly applied in simple box'es content, but this is the same case as other styles (link is a special kind of style in fact).

Yup, the Link feature is disabled for the nested editables in simple box, but it is still enabled for the entire widget (i.e. its outer element). But when I used it, nothing happened, so to say, as the link wasn't really added.

comment:10 Changed 5 years ago by Piotrek Koszuliński

Yup, the Link feature is disabled for the nested editables in simple box, but it is still enabled for the entire widget (i.e. its outer element). But when I used it, nothing happened, so to say, as the link wasn't really added.

Ok, we've been checking different simple box versions. The one we recently added in http://ckeditor.dev/plugins/widget/dev/nestedwidgets.html allows links inside its content. But in general - link will be disabled when entire simple box is selected, because link is a style and simple box is a block widget.

However, link (and other styles) will be still enabled on placeholder and mathjax widgets because they are inline widgets. And image2 is a special case, because it handles link in a totally custom way - the link wraps only <img> not entire <figure> or contents of <figcaption>.

comment:11 Changed 5 years ago by Piotrek Koszuliński

Status: pendingconfirmed
Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy