Skip to content
This repository has been archived by the owner on Jan 15, 2019. It is now read-only.

Display of right-aligned images is inconsistent between the front and back end #696

Closed
kjellr opened this issue Nov 30, 2018 · 5 comments
Closed
Labels
bug Something isn't working has approved PR
Milestone

Comments

@kjellr
Copy link
Collaborator

kjellr commented Nov 30, 2018

Description

Twenty Nineteen is designed to have right-aligned elements extend outside of the text column like so:

screen shot 2018-11-30 at 3 24 47 pm

For image blocks, this occurs in the editor as intended. But on the front end, the image is tucked all the way into the text column instead:

screen shot 2018-11-30 at 3 12 55 pm

Details

This happens because our standard alignright block styles target direct children of .entry-content. For most blocks (Cover, Pullquote, Galleries,etc.), that works just fine. They apply the alignright class to their topmost container like so:

<div class="entry-content">
  <figure class="wp-block-pullquote alignright"> ... </figure>
</div>

By contrast, Left- and right-aligned images are wrapped in a container div that does not include the alignment class:

<div class="entry-content">
   <div class="wp-block-image">
    <figure class="alignright"> ... </figure>
  </div>
</div>

This prevents the image from picking up our alignment styles.

Possible Solutions

  1. A change like Try: Alternate floats markup gutenberg#10394 in Gutenberg would fix this by bringing the alignright classname to the wrapper div. This would also make this behavior more consistent with the way that classname appears for other blocks. (There's some background in there regarding why that may not be a good idea though)
  2. On our side, we could adjust the editor style so that it keeps the floated item within the text column. This would at least bring consistency to what the user sees in the front and back end. This would be a simple change, but it'd still be a little weird that all other right-aligned blocks will float outside of the text column. 😕

cc @jasmussen for some thoughts, since you know a lot of the background on this one. 🙂

Note: that this issue a little similar to #688, which occurs because non-captioned images are wrapped in a p tag when inserted via the Classic Editor.

@kjellr kjellr added the bug Something isn't working label Nov 30, 2018
@kjellr kjellr added this to the 5.0.1 milestone Nov 30, 2018
@jasmussen
Copy link
Collaborator

Sorry about the headaches here.

The solution that we arrived at was the one that was both the most compatible with existing themes, ironically, and the one that we felt provided the most flexibility for themers. This does not mean it cannot be revisited — though I think perhaps floats could be revisited holistically in the future, they really do cause challenges especially to grid based layouts.

However a question: is it not possible to leverage the markup present, and still "pull out" the image on the right? Does it require a markup change to pull it out on the frontend like it is in the editor?

@kjellr
Copy link
Collaborator Author

kjellr commented Dec 3, 2018

However a question: is it not possible to leverage the markup present, and still "pull out" the image on the right? Does it require a markup change to pull it out on the frontend like it is in the editor?

Good thinking... My Friday afternoon brain thought that would be too difficult, but my Monday morning brain got that mostly working within 10 minutes. 😄 Opened a PR here: #702

@joyously
Copy link

joyously commented Dec 3, 2018

The PR 702 looks like it just affects image blocks, and not legacy images that are alignleft and alignright.
Is that intentional?

@kjellr
Copy link
Collaborator Author

kjellr commented Dec 3, 2018

The PR 702 looks like it just affects image blocks, and not legacy images that are alignleft and alignright.
Is that intentional?

That's right. Legacy right-aligned images w/captions already behave as intended. Legacy right aligned images with no captions do not, but they would need a different fix.

@kjellr
Copy link
Collaborator Author

kjellr commented Jan 8, 2019

Closing since #702 has been merged.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working has approved PR
Projects
None yet
Development

No branches or pull requests

3 participants