Problem/Motivation

In some cases it is necessary to use a certain heading's visual style but it is not possible to change the markup to use the correct element and it's styles.
For example displaying what is perceived as a h2 heading when you can only print a h3.
So it would be nice to have this possibility in the theme to achieve this with reusable heading classes.

Seven has already carried out this work here #2028053: Add typographic styles, components, and utility classes and explains the point of this very well. And we should use add this to Bartik too.

The work is needed for part of this issue.... #1768732: Wrong document outline because of Term reference label in h3.

The suggestion for this work is outlined in this Bartik issue... #2398445: Clean up the "elements" component in Bartik.

Proposed resolution

Add reusable heading classes to each heading element (h1, h2, h3 etc) declared in Bartik.
I suggest using the same style classes that Seven has, so .heading-a, heading-b etc. See here #2028053: Add typographic styles, components, and utility classes.

Remaining tasks

Write a patch,
Review the patch
Manual testing.
Add screenshots to show what the patch does visually.

User interface changes

API changes

CommentFileSizeAuthor
#11 interdiff-2400233-9-11.txt774 bytesemma.maria
#11 bartik-reuseable-header-classes-2400233-11.patch1.02 KBemma.maria
#9 interdiff.txt557 bytesKatiemouse
#9 2400233-9-reusable-heading-classes.patch1.07 KBKatiemouse
#4 2400233-4.patch833 bytesrpayanm
#4 2400233-interdiff.txt937 bytesrpayanm
#1 bartik-reuseable-header-classes-2400233-1.patch821 byteslewisnyman

Comments

lewisnyman’s picture

lewisnyman
he/him
commented
Status: Active » Needs review
StatusFileSize
new bartik-reuseable-header-classes-2400233-1.patch821 bytes

Added the same classes as Seven.

emma.maria’s picture

emma.maria
she/her
commented
Title: Add reusable header classes to Bartik » Add reusable heading classes to Bartik
emma.maria’s picture

emma.maria
she/her
commented
Status: Needs review » Needs work

Seven's classes are worded wrong so we need to add heading-a heading-b etc to the patch in this issue, I've also raised this issue for Seven :) #2402061: Reusable heading classes should be "heading-*" not "header-*".

rpayanm’s picture

rpayanm
he/him
Primary language Spanish
commented
Status: Needs work » Needs review
StatusFileSize
new 2400233-interdiff.txt937 bytes
new 2400233-4.patch833 bytes
lewisnyman’s picture

lewisnyman
he/him
commented
Status: Needs review » Reviewed & tested by the community

Looks good to me, not much else to test here.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work

Can we add a comment about the fact that the heading-x classes are not used so that an eagle-eyed contributor does not remove them.

emma.maria’s picture

emma.maria
she/her
commented

Yes sure.

@LewisNyman you are better at technical wording so you should really draft the sentence but I have come up with...

"Heading-x classes are additional useful classes to be used in cases when you have to mimic the styles of a particular element for a different element."

If this is OK? I will add it to the patch.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Let's make it match whatever we add to #2402061: Reusable heading classes should be "heading-*" not "header-*"

Katiemouse’s picture

Katiemouse commented
Status: Needs work » Needs review
Issue tags: +CatalystAcademy
StatusFileSize
new 2400233-9-reusable-heading-classes.patch1.07 KB
new interdiff.txt557 bytes

Hi, I am in high school and new to drupal. I put up a patch for the other issue: #2402061: Reusable heading classes should be "heading-*" not "header-*" and was suggested to do the same for this one. My attempt is attached :)

lewisnyman’s picture

lewisnyman
he/him
commented
Status: Needs review » Needs work

Copying over the same feedback from: #2402061: Reusable heading classes should be "heading-*" not "header-*"

  1. +++ b/core/themes/seven/css/base/elements.css
@@ -36,37 +36,44 @@ h3,
+ * Classes '.heading-a' - '.heading-f' are not currently in use; they are still
+ * included in stylesheet to keep Seven and Bartik consistent and good examples
+ * of easy to configure base styles.

    I am being picky but I would like to change the comment here. I think they will be used in the future so maybe we should just define their purpose instead of just that they aren't used.

    How about:
    "Reusable heading classes are included to help modules change the styling of headings on a page without affecting accessibility." 

  2. +++ b/core/themes/seven/css/base/elements.css
@@ -36,37 +36,44 @@ h3,
+ * @see https://www.drupal.org/node/2400233

    I don't think we need to link to the other issue.

Also, I've never seen us place a comment between selectors, can we put it above the h1, h2, h3 selectors please? Thanks!

emma.maria’s picture

emma.maria
she/her
commented
Status: Needs work » Needs review
StatusFileSize
new bartik-reuseable-header-classes-2400233-11.patch1.02 KB
new interdiff-2400233-9-11.txt774 bytes

This issue hasn't been picked up in a while.

Here's the new patch as per instruction in #10. I have amended the comment content and moved the comment above the selectors plus added a space above the comment as per the CSS formatting standards.

Plus interdiff.

lewisnyman’s picture

lewisnyman
he/him
commented
Status: Needs review » Reviewed & tested by the community

Looking goooood

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

CSS changes are permitted in beta. Committed 07b3f0a and pushed to 8.0.x. Thanks!

  • alexpott committed 07b3f0a on 8.0.x 
    Issue #2400233 by emma.maria, rpayanm, Katiemouse, LewisNyman: Add...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.