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

Table of Contents / Outline: Treat Cover Images as H2-level headings #4616

Closed
wants to merge 1 commit into from

Conversation

mcsf
Copy link
Contributor

@mcsf mcsf commented Jan 20, 2018

Fixes #4606

Description

I don't know if this is functionality that we want. I think it makes sense to have, in some form or another, Gutenberg recognizing cover images as some form of document hierarchy. Feel free to close this PR and its parent issue if this isn't the way to go.

Caveat: #3444 is in progress and might change the circumstances. The present PR may be a temporary improvement. I also think it conceivable that Cover Image would offer different heading levels.

Screenshots (jpeg or gifs if applicable):

screen shot 2018-01-20 at 12 36 00

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code has proper inline documentation.

@mcsf mcsf added [Type] Enhancement A suggestion for improvement. [Type] Question Questions about the design or development of the editor. Needs Design Feedback Needs general design feedback. labels Jan 20, 2018
@mcsf mcsf requested review from mtias and jasmussen January 20, 2018 12:39
@jasmussen
Copy link
Contributor

Nice! I think this one should definitely be counted, don't you agree @afercia?

I reckon that the Cover Image, while it can be used for decoration only, will often be used in the same way as a heading would.

It also seems to work largely as intended:

screen shot 2018-01-22 at 08 50 45

One questions though: should a heading be counted if you do not type anything into the H2? Or to phrase it differently, should the Cover Image block not output the H2 tag into the markup if you don't type anything?

A different question surfaces: right now a placeholder block for the Cover Image actually outputs a gray square in the saved source. Should the placeholder output anything at all? And if we made it not output anything, should the placeholder not count towards the headings?

screen shot 2018-01-22 at 08 50 18

@afercia
Copy link
Contributor

afercia commented Jan 22, 2018

Hm, not sure. #3444 is going to change the h2 to a p because we can't predict ho users will use a cover image. Headings should identify a section of content. What it the cover image is used just visually and immediately followed by another heading? In this case, the fact the cover image outputs a heading would be pointless, as it doesn't identify any content. /cc @samikeijonen

Id also like to remind there shouldn't be any hardcoded heading level in the output. Themes may use a different heading hierarchy and we can't assume a h2 will fit in the hierarchy. Ideally, as WordPress does, any HTML output should be filterable...

should the Cover Image block not output the H2 tag into the markup if you don't type anything?

Empty headings are evil 🙂

@samikeijonen
Copy link
Contributor

I'd also like to remind there shouldn't be any hardcoded heading level in the output. Themes may use a different heading hierarchy and we can't assume a h2 will fit in the hierarchy.

This was the reason why #3444 uses <p>.

And +100+1 for not outputting any empty markup.

@jasmussen
Copy link
Contributor

Converting the H2 to P is fine so long as it visually is unchanged. Same for not outputting empty markup.

In both those cases, it seems like we have an answer to this PR, though, which is to not count the Cover Image as a heading.

Thanks a whole bunch for exploring this, though, Miguel! 🏅

@mcsf
Copy link
Contributor Author

mcsf commented Jan 23, 2018

My pleasure!

@mcsf mcsf closed this Jan 23, 2018
@mcsf mcsf deleted the add/document-outline-cover-image-support branch January 23, 2018 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Design Feedback Needs general design feedback. [Type] Enhancement A suggestion for improvement. [Type] Question Questions about the design or development of the editor.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cover Image H2 does not appear in Table of Contents
4 participants