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

Try extra wrapping element around floated images #5460

Closed
wants to merge 1 commit into from

Conversation

jasmussen
Copy link
Contributor

@jasmussen jasmussen commented Mar 7, 2018

This adds a wrapping DIV around a floated image. This makes it easier for themers to float images with captions in wide and fullwide layouts.

This description has been edited for clarity.

This adds a wrapping DIV around a floated image. The reason being — the `figure` element cannot be a child of a `p`, and when you combine that with a `figcaption`, it's hard to create layouts that accommodate both wide and floated images.

This PR is an experiment.
@jasmussen
Copy link
Contributor Author

This fixes #5292.

@jasmussen
Copy link
Contributor Author

Would this PR be more digestible if the wrapping element was only added if a theme opted in to wide and fullwide styles? The wrapping element is only really necessary in that environment in the first case.

@jasmussen jasmussen self-assigned this Apr 4, 2018
@jasmussen jasmussen removed the [Status] In Progress Tracking issues with work in progress label Apr 4, 2018
@aduth
Copy link
Member

aduth commented Apr 4, 2018

The reason being — the figure element cannot be a child of a p

Can you explain in what context it would be a child of p?

@jasmussen
Copy link
Contributor Author

jasmussen commented Apr 4, 2018

Can you explain in what context it would be a child of p?

Any child of the p element can be considered an "inline" element, and it was suggested at one point that floats could be inline elements (to inherit the width of the text, in wide environments). But then they can't have the figure element.

@aduth
Copy link
Member

aduth commented Apr 10, 2018

But I don't understand when the image block would be a descendent of p.

@jasmussen
Copy link
Contributor Author

But I don't understand when the image block would be a descendent of p.

Well in this PR, that wouldn't be the case as I'm wrapping it in a DIV instead.

The reason for using a P, would be to make layouts like suggested in https://codepen.io/joen/pen/oEYVXB somewhat more simple, as we can presume that the P element has a centering & max width style on it. I recognize this would not be ideally semantic.

@jasmussen jasmussen added the [Status] In Progress Tracking issues with work in progress label Apr 11, 2018
@jasmussen
Copy link
Contributor Author

Adding back the "In Progress" label, as I realized that this PR will become moot if #5973 gets merged in without big changes. So it should sort of be one or the other.

@jasmussen
Copy link
Contributor Author

Closing, as #5973 which is the other approach, has been merged.

@jasmussen jasmussen closed this Apr 13, 2018
@jasmussen jasmussen deleted the try/floats-in-extra-divs branch April 20, 2018 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] In Progress Tracking issues with work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants