Media Library: Default value for data-align attribute should not be center

Created on 10 December 2019, about 5 years ago
Updated 8 February 2023, almost 2 years ago

Problem/Motivation

Media Library: Default value for data-align attribute should not be center. Why? From #12 Media Library: Default value for data-align attribute should not be center Fixed :

We most often translate "left" into float: left;, "right" into float: right; and "center" into margin: auto. With "none", we take the item as-is, and have it flow normally based on its size and the text alignment of the container. So, at least for us, "center" is a conscious decision on how the item should be displayed, thus it shouldn't be the default.

Also, why would there be a "None" setting, if we consoder "center" as the "default" (i.e. unchanged) state?

Steps to reproduce

Embed a media item -- any media item -- into a WYSIWYG text area, leaving the default settings in place.

Proposed resolution

As per comment #4 and #5, the default value should be left to none.

Remaining tasks

  • Create tests

User interface changes

When embedding media items in WYSIWYG, they will have no alignment by default.

API changes

None.

Data model changes

None.

Release notes snippet

TBD

Feature request
Status

Fixed

Version

10.1

Component
Media 

Last updated about 1 hour ago

Created by

🇺🇸United States mel-miller Oakland, CA

Live updates comments and jobs are added and updated live.
  • CSS

    It involves the content or handling of Cascading Style Sheets.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • 🇺🇸United States ricksta

    Comment #50 is correct that this commit introduced a regression in the CKEditor interface. I've removed the margin and text-align code as per Wim Leers suggestion and it does fix the issue he pointed out. First screenshot shows after the last commit was made.

    This screenshot shows the results of the code change to .ck .drupal-media demonstrating the image is left-aligned and visually appears left-aligned.

    The screenshot below shows that the text-aligning to center is still working and there are no regressions.

    I should note that there are margins left and right of 40px that make it look like when the image is left-aligned or right-aligned, that the image is not flush with the editing window. These margins come from the user-agent style sheet. Should we override this behavior?

  • Status changed to RTBC almost 2 years ago
  • 🇺🇸United States mark_fullmer Tucson

    I have verified manually that the CSS change proposed does cause the default alignment of a media item to appear left-justified, and that center and right-justification behave as expected in the context of CKEditor 5's editing window.

    As for the question about the remaining margin, visible in the second screenshot above, it's tricky: that is coming from the figure element of the drupal-media downcasted version of the element, and would not usually be present when the media item is rendered on the page. So, the margin is potentially misleading, but it's also impossible to predict whether the frontend theme will add a similar margin for the media item or not. I think the presence of the margin doesn't cause confusion about alignment, which was the original problem. So I think it should be left as-is, and sites can add CKEditor-specific CSS if they want to change it.

  • 🇺🇸United States xjm

    Fixing attribution.

    I was confused to see this pop up again -- in general, when we hotfix things, the issue should either be reverted or a followup filed. We shouldn't reopen fixed issues without a revert.

    Anyway, committed the hotfix to 10.1.x. Thanks!

  • Status changed to Fixed almost 2 years ago
    • xjm committed 1b41f3ba on 10.1.x
      Issue #3099878 followup by Wim Leers, ricksta, mark_fullmer
      
  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024