Problem/Motivation
There currently is no way to unset data-align from images or media using the UI! This is a regression compared to CKEditor 4.
Remaining tasks
We have three options how this could be handled in the UI:
- Implement button for the default style
- Remove
data-alignwhen the currently active style button is clicked - Follow CKEditor 5's lead and provide explicit choices rather than toggling states (see #5 + #8)
This MR implements option 3.
User interface changes
- Before

- After


API changes
None.
Data model changes
N/A
Release notes snippet
N/A
| Comment | File | Size | Author |
|---|---|---|---|
| #43 | 3264727-43-93x.patch | 80.95 KB | bnjmnm |
| #39 | 3264727-39-d9.patch | 80.94 KB | lauriii |
| #39 | 3264727-39-d10.patch | 80.78 KB | lauriii |
| #35 | 3264727-35-d9.patch | 80.33 KB | bnjmnm |
| #35 | 3264727-35-d10.patch | 80.17 KB | bnjmnm |
Issue fork drupal-3264727
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
wim leersI noticed something similar. Since the very beginning inserted media were getting
data-align="center". I don't know why.I like option 2 best.
Comment #3
lauriiiComment #5
lauriiiImplement approach where there is a new button called "Break text". That is more inline with the default CKEditor implementation + approach implemented by Google Docs. On top of that, images have an option to make an image inline.
Comment #6
wim leersHm … the UX works as expected, but I am really confused by the button label. I would expect:
as button labels, not . Maybe that's just me? 🤔 I have never seen "break text" before.
The icon is clearer than the label IMHO.
Will compare with similar UXes tomorrow.
Comment #7
wim leersI still can't make images inline, if that's what you mean? Can you elaborate?
Comment #8
wim leersI … am not sure. I was pretty confused by "break text" — I had no idea what it meant. It only makes sense when somewhere in the UI it says "wrap text" too, and that really is only the case in Google Docs. Apple Pages names it differently. Microsoft Word (which I do not have available) uses again different terminology (based on a DuckDuckGo image search).
This is true. But it's not exactly the same.
My conclusion: every rich text editor does this differently. Therefore there is no clear best choice. I personally think that the label is confusing, and either or would be clearer. But if you feel strongly about the current approach, I'm fine with that too.
Comment #9
wim leers19b3c464 basically is exactly what the CKEditor 5 text editor does at https://ckeditor.com/ckeditor-5/demo/#document. The other demos that they have there are tweaked to have a "side image" image style, which is always right-aligned (left aligning is not supported in those demos).
Discussed in detail by 3 people: @lauriii, @nod_ and I. We reached the conclusion that A) there is no consistency at all between different rich text editors, B) "center align with wrapping text" is not supported by Drupal's
data-alignfilter, which arguably is confusing/inconsistent, but it's been working this way for many years now. Changing that would definitely result in a BC break.Therefore it seems reasonable to move forward with a solution that:
We can then refine this further in the future.
Comment #10
wim leersComment #11
wim leersComment #13
bnjmnmI agree that this is an improvement over HEAD and that justifies adding as-is, but I do have concerns about the wording that I'd like explored in a stable blocking follow-up. Perhaps we could bring it to a UX meeting. That doesn't need to block the feature addition here, though.
I also noticed the tooltip provides different wording for effectively the same button depending on it being whether its directly in the toolbar or a dropdown. This would be a consistency concern regardless, but it seems especially important here as the icons are kind of difficult to visually distinguish already.
Comment #14
bnjmnmNot sure I like the dropdowns either - I'd rather it be a single dropdown or all in the main toolbar. It doesn't necessarily need to be in this issue's scope but I wanted to get that documented here too.
Comment #15
wim leersI noticed this too. There's less text in the tooltip if you're already in the context of a specific dropdown. I think this is okay? This is the way CKEditor 5's dropdowns work.
"All in the main toolbar" is exactly what @lauriii had in his first iteration and what I opposed. It becomes an enormous toolbar with an overwhelming number of choices.
I personally also still prefer a single dropdown. But @lauriii has some solid concerns about that because not all alignments work in the same way, AND for images there's also the "inline" functionality.
That being said … I think that a single dropdown for both images and media, and descoping the "inline image" button from this issue might be the most elegant way forward nonetheless. IMHO presenting all of these in a single dropdown is still the least amount of mental overhead, and the effects of any of the choices are clearly observable.
Curious what @lauriii thinks 🤓
Comment #16
lauriiiLeaving inline image out from this issue would leave some gaps to the UX. It would be the only state where image alignment is not visible through the UI. I think it should be part of the scope to be able to remove
data-alignto be either a block image or an inline image since inline image is a valid state in our configuration at the moment. We could have a follow-up issue to discuss if we want to disable inline images if that's a concern.I'm fine with a single dropdown (that's what I implemented as the first iteration) but I did personally find this solution was better at explaining the nuance between different alignment options (break text vs wrap text). That was something that we struggled to visualize with Wim when we tried to compare our alignment options to other systems.
I'm not too keen to any of these solutions so if people strongly believe that a single dropdown is a better solution, I'm happy to change back to that, or anything else that folks have preference on.
Comment #25
bnjmnmI presented the tooltip naming and button location to the Feb 25 UX meeting: #3265255: Drupal Usability Meeting 2022-02-25. It was an active and productive conversation.
Broadly:
Everyone was +1 or neutral on adding separate buttons:
A few additional things:
The "in line" wording was not discussed. I'd still like that to happen at some point but it doesn't have to be in this issue.
Comment #29
benjifisherFor the record, the attendees at today's usability meeting were andregp, Antoniya, AaronMcHale, ckrina, bnjmnm, kimberlly_amaral, worldlinemine, victoria-marina, tmaiochi, rkoller, shaal, and me.
It looks as though @bnjmnm credited most of us before Comment #25. I will add credits for the ones he missed.
The issue for the usability meeting (link in #25) will have a link to the recording and a transcript.
Comment #30
lauriiiThank you everyone for the input! I'm personally fine with moving all of the alignment options to the toolbar but that does mean that we need to come up with an alternative solution to #6 which the split buttons tried to solve.
Here's the list of states that we need to support for backwards compatibility:
data-align, block<img>data-align, inline<img>data-align="left", block<img>data-align="right", block<img>data-align="center", block<img>The option that we found particularly difficult to explain is
No alignment / Block image. This was called "Break text" in the first iteration, which @Wim Leers found confusing. Do we want to go with the tooltip "Break text: no alignment"? If we explicitly mention break text there, should we also have an explanation on other buttons whether they wrap or break text?Comment #31
lauriiiHere's a screenshot of how the toolbar looks when all of the buttons are displayed directly in the toolbar:

The previous UI was directly tied to the data-align attribute where the no data-align attribute option was called "Align: none". I personally don't think that "No alignment" is a good label for alignment since technically the element needs to always be aligned somehow. That only makes sense when you understand the internal representation of the alignment, which is
data-align. For that same reasontext-alignproperty in CSS cannot be set tonone. That's why I proposed using "Break text" as the label of that state. However, that might not be the best label since that's describing its relationship to text, which other alignment options except "In line" doesn't do.Edit: I guess another problem with "No alignment" is that it doesn't explain how it's different from "In line", since technically "In line" would also not have "alignment" since it doesn't have
data-align.Comment #32
wim leersAfter so much careful iteration and balancing of different aspects, I feel like we've arrived at a very good balancing point.
When @lauriii, @bnjmnm, @hooroomoo and I discussed this yesterday, I felt @bnjmnm made a crucial point: it is very empowering for the user to be able to toggle between different options in a fast and simple manner. Then concerns about making the labels perfectly understandable — while still important — matter less. Especially in a context like this, where no text editing software in the world is conveying this perfectly, hence they all do it differently.
This is without a doubt a leap forward. 😊🚀
Comment #33
bnjmnmVery minor feedback in MR, enough that it can be switched back to RTBC after it is addressed in my opinion.
Comment #34
lauriiiResponded to feedback on MR, moving to RTBC for visibility.
Comment #35
bnjmnmPrepared to commit when the version-specific patches are green
Comment #38
bnjmnmI suspect tests need to be updated due to #3264775: [drupalMedia] Toolbar should be visible when element inside <drupalMedia> is focused being committed since tests last ran for this issue.
Comment #39
lauriii@bnjmnm that was indeed the reason for the broken tests 😅 fixed the conflicts and tests should pass now.
Comment #42
bnjmnmGreat work and navigation of tricky UX issues. Committed to 10.0.x and 9.4.x, and intend to add to 9.3.x (since CKEditor 5 is experimental) when the production branch freeze is over.
Comment #43
bnjmnmComment #45
bnjmnmCommit freeze is over, so ported to 9.3.x.