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

Wrap cite tag with footer in Quote and Pullquote blocks #10465

Closed
wants to merge 1 commit into from
Closed

Wrap cite tag with footer in Quote and Pullquote blocks #10465

wants to merge 1 commit into from

Conversation

ZebulanStanphill
Copy link
Member

Description

Previously #9804.

This PR changes the Quote and Pullquote blocks to wrap their <cite>s with <footer>s. The reason for this is that <cite> tags can be used within a quote but not be the citation of a quote, e.g. using a <cite> element to refer to a book or ship. The use of a <footer> improves the semantics to show that the contained <cite> is the citation of the entire <blockquote>, and makes it easier for themes to style inline <cite>s differently from the one in the <footer>.

In the editor, the RichText field for the citation is a <footer> element, where previously it was a <div> due to #8785 and before that it was a <cite>.

How has this been tested?

I have tested to make sure that old Quote and Pullquote blocks are automatically converted properly. Some errors appear in the JavaScript console the first time a post is opened containing the deprecated forms of the blocks, but I am guessing that is due to the deprecated forms not matching the other even older deprecated forms before matching the last one in the list. The content is preserved and automatically converted as expected and no blocks are made invalid. After saving, no errors appear in the JavaScript console.

(I would say that the failed matches of deprecated forms should not be called out in the JavaScript console as errors, but I am not sure of the history of this functionality or how it works, and that is out-of-scope for this PR anyway.)

Additional info

@chrisvanpatten
Copy link
Contributor

This makes sense to me, but could you elaborate more on the backcompat aspect here in terms of styling? I would guess it's pretty minimal but just to be sure this markup change won't cause themes unexpected issues before they're able to adjust their CSS.

@@ -35,12 +35,12 @@
font-size: 32px;
}

.wp-block-pullquote__citation {
footer {
Copy link
Member Author

Choose a reason for hiding this comment

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

The switch to not using a class here makes the CSS simpler and easier to override, but it may cause minor styling issues for themes that were using this class to style blocks. I'm not sure how long the class has been around, so it may be that there are few or even no themes that actually use it. I could apply styles to both and add the class back into the markup and deprecate it, if desired.

I'd also appreciate some testing to make sure everything looks the way it should compared to master.

@ZebulanStanphill
Copy link
Member Author

I think this is something that should probably be resolved before WP 5.0 due to the effects it has on both the editor and front-end markup + styling. Could this be milestoned for 4.1 or one of the other pre-WP-5.0-release milestones?

@ZebulanStanphill
Copy link
Member Author

Closing in favor of #11695. I plan to make a separate PR later for the Pullquote block.

@ZebulanStanphill ZebulanStanphill deleted the update/wrap-cite-with-footer-in-quote-and-pullquote-2 branch November 9, 2018 19:47
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