Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get floated images working as expected. #59

Merged
merged 3 commits into from
Jun 8, 2018

Conversation

kjellr
Copy link
Collaborator

@kjellr kjellr commented Jun 7, 2018

Currently, floated images are broken:

  • They have giant margins.
  • Resized images look really tiny.
  • Captions don't share the same margins as the images themselves, so text wraps oddly around them.

gutenberg test__p 24

This PR fixes that:

gutenberg test__p 24 1

There is one caveat though — The theme imposes a 50% width on all floated images, as per @jasmussen's commits in #40. Unfortunately, this means that we ignore the custom dimensions of all images. :/

We can theoretically remove this and everything works mostly fine. The only remaining bug is the image captions. I can't find a graceful way to make those captions mirror the width of the image above them:

gutenberg test__p 24 2

@jasmussen (or anyone else), I wonder if you have any suggestions for how we should handle this?

kjellr added 3 commits June 7, 2018 10:27
…t. This is required for them to take up the avaialble space when floated.
Set content width to 636 instead of 740 (to align with changes in WordPress#49)

Add margins to figcaptions in floated images so they match the margins of the images themselves.
Not just resized ones. Turns out they need to be 100% wide for Chrome to display properly.
@jasmussen
Copy link
Contributor

Confirmed, this is working! Thanks so much.

@jasmussen jasmussen merged commit b0b9fee into WordPress:master Jun 8, 2018
@kjellr kjellr deleted the update/floated-images branch June 8, 2018 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants