Updated: Comment #0
Problem/Motivation
#2170085: Caption filter may not wrap images that are only aligned, not captioned fixed a major problem. But it kind of introduced a minor new one: we introduced generic alignment classes for non-textual content. That's fine, or great even.
The problem we subtly introduced, was that we didn't remove the caption alignment classes. So now we not only have align-(left|center|right), but we also still have caption-(left|center|right).
This is problematic when you take into account that the CKEditor widget that provides a great authoring UX for aligning/captioning images also has to know about these different classes: if the image is captioned, it must set caption-(left|center|right), if it's not captioned, it must set align-(left|center|right). That's unnecessary complexity. Therefor it complicates #2039163: Update CKEditor library to 4.4.
Proposed resolution
Get rid of the caption-(left|center|right) classes.
Remaining tasks
None.
User interface changes
None.
API changes
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | remove_caption_alignment_classes-2180627-7.patch | 6.63 KB | jessebeach |
| #7 | reject-bit.txt | 2.02 KB | jessebeach |
| #3 | interdiff.txt | 575 bytes | jessebeach |
| #2 | interdiff.txt | 5.95 KB | jessebeach |
| #2 | remove_caption_alignment_classes-2180627-2.patch | 5.95 KB | jessebeach |
Comments
Comment #1
wim leersComment #2
jessebeach commentedI think we can get away with removing this modification:
If we remove the
margin: 0from the.captionclass. I read through the referenced article http://stackoverflow.com/a/13363408 and I don't believe themargin: 0is strictly necessary.Comment #3
jessebeach commentedHere's the correct interdiff for #2
Comment #4
wim leersYou're right, the
margin: 0is non-essential, if it's necessary, it should be done at the theme level.Now
filter.caption.cssfinally is as clean as it should be :)Manually tested. Still working correctly.
Comment #5
jessebeach commentedHere are issues to style
.captionelements in Bartik and SevenBartik: #2183005: Style inlined .caption elements (figure) - Bartik
Seven: #2182997: Style inlined .caption elements (figure)
Comment #7
jessebeach commentedI have no idea what changed in Drupal to make that patch no longer apply. I pulled out the rejected bits so you can see them.
Comment #8
wim leersThis was broken by #1913510: Core Javascript Files Not Using Standard Indentation. Patch is functionally the same. Back to RTBC.
Comment #9
webchickCommitted and pushed to 8.x. Thanks!
Comment #10
nod_Change to a JS file. Tagging.
Comment #11
wim leers