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

Restore captions for image and embed #7749

Merged
merged 1 commit into from
Jul 6, 2018
Merged

Restore captions for image and embed #7749

merged 1 commit into from
Jul 6, 2018

Conversation

jasmussen
Copy link
Contributor

This PR is a followup to #7624.

It adds back the caption styles for images and embeds. This is because the majority of existing themes won't have styles to accommodate the new figcaption markup, and are likely to be styling the WP Captions instead. For those themes, if the caption styles live in the theme.scss file, the captions will appear unstyled or broken.

Some time in the future, this can possibly be revisited and shuffled around.

screen shot 2018-07-06 at 17 13 57

@jasmussen jasmussen added the [Type] Bug An existing feature does not function as intended label Jul 6, 2018
@jasmussen jasmussen added this to the 3.2 milestone Jul 6, 2018
@jasmussen jasmussen self-assigned this Jul 6, 2018
@jasmussen jasmussen requested a review from mtias July 6, 2018 15:16
Copy link
Member

@tofumatt tofumatt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick question about deprecation thoughts on this one, but makes sense.


.wp-block-embed {
// Supply caption styles to images, even if the theme hasn't opted in.
// Reason being: the new markup, figcaptions, are not likely to be styled in the majority of existing themes,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this intended to be deprecated eventually, once themes know about this new markup? Or will we keep it?

This comment reads a bit like it's a stop-gap.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should phase it away, but don't want to rush it because it will cause a visual degradation at this moment.

This PR is a followup to #7624.

It adds back the caption styles for images and embeds. This is because the majority of existing themes won't have styles to accommodate the new `figcaption` markup, and are likely to be styling the WP Captions instead. For those themes, if the caption styles live in the `theme.scss` file, the captions will appear unstyled or broken.

Some time in the future, this can possibly be revisited and shuffled around.
@aduth aduth force-pushed the fix/add-captions branch from 4fa9db9 to d677a7a Compare July 6, 2018 16:07
@aduth
Copy link
Member

aduth commented Jul 6, 2018

Rebased because there was a failing preview end-to-end test, which could have been impacted (improved?) by the more recent merge of #7703.

@aduth aduth merged commit 6edaf68 into master Jul 6, 2018
@aduth aduth deleted the fix/add-captions branch July 6, 2018 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants