Make WordPress Core

Opened 4 years ago

Closed 22 months ago

Last modified 22 months ago

#52028 closed enhancement (fixed)

Twenty Fifteen: H5 and H6 headings have the same font size

Reported by: poena's profile poena Owned by: audrasjb's profile audrasjb
Milestone: 6.1 Priority: normal
Severity: normal Version: 4.1
Component: Bundled Theme Keywords: good-first-bug has-patch has-screenshots commit
Focuses: css Cc:

Description

This was originally reported by a user via the report button in the theme directory:

there is one 'elements' issue, namely, Header 6.
When using Header 6, it appears to be as the same size with Header 5 (both same size?)

-Confirmed that both H5 and H6 headings have a default font size of 17px.
Headings should be styled to indicate their importance, where a lower heading level has a smaller size.

Because the default paragraph font size is also 17px, a solution would be to increase
the font size of the H5 heading.

Attachments (6)

52028.diff (691 bytes) - added by akabarikalpesh 4 years ago.
I added h5's font size is 18px. let me know if any changes.
52028.1.diff (2.4 KB) - added by afrid1719 3 years ago.
I have increased the font-size for H5 by 1px for all devices having screen size greater than or equal to 740px.
52028.patch (2.3 KB) - added by aezazshekh 3 years ago.
I have increased the tag size by 1pixel for all screen sizes. Please let me know if you have any query about the same.
52028.2.patch (5.2 KB) - added by viralsampat 2 years ago.
I have changed the tag font size to 20 pixels for desktop and also made respective changes for responsive devices. Please let me know if you have any query about the same.
52028.2.diff (696 bytes) - added by pratiweb 2 years ago.
Increase H5 font size to 18px for all content.
52028.3.diff (4.2 KB) - added by russel07 2 years ago.
I have updated the font size and margin properties in several places.

Download all attachments as: .zip

Change History (34)

@akabarikalpesh
4 years ago

I added h5's font size is 18px. let me know if any changes.

#1 @akabarikalpesh
4 years ago

  • Keywords has-patch needs-testing added
  • Version set to 5.6

#2 @hellofromTonya
4 years ago

  • Version changed from 5.6 to 4.1

The styling in the theme was introduced in 4.1 in this commit.

@afrid1719
3 years ago

I have increased the font-size for H5 by 1px for all devices having screen size greater than or equal to 740px.

@aezazshekh
3 years ago

I have increased the tag size by 1pixel for all screen sizes. Please let me know if you have any query about the same.

#3 @sabernhardt
3 years ago

  • Keywords needs-refresh added; needs-testing removed
  • Milestone changed from Awaiting Review to 6.0

Thanks for the patches!

When editing the font size, at least in this theme, the line-height and margin values should change as well.

The values for 20px and 18px are already in the stylesheet:

	font-size: 20px;
	font-size: 2rem;
	line-height: 1.4;
	margin-top: 2.8em;
	margin-bottom: 1.4em;
	font-size: 18px;
	font-size: 1.8rem;
	line-height: 1.3333;
	margin-top: 2.6667em;
	margin-bottom: 1.3333em;

And this is an option for 16px:

	font-size: 16px;
	font-size: 1.6rem;
	line-height: 1.25;
	margin-top: 3.25em;
	margin-bottom: 1.625em;
Last edited 3 years ago by sabernhardt (previous) (diff)

This ticket was mentioned in PR #2311 on WordPress/wordpress-develop by anitanenova.


2 years ago
#4

  • Keywords needs-refresh removed

To be more easy to show you the result of my changes I added in a custom post 3 heading with h4, h5, and h6 HTML tag. The following changes are:

  • for media query min-width: 77.5em: I had to change the font-size to h6 HTML too because the values of h4 and h6 don't give us a big range to we can find suitable value for h5 tag. The result is: https://ibb.co/fkf01QP ;
  • for media query min-width: 68.75em: the change here is for h5 tag and the result now has to have this look https://ibb.co/w6hRxg0;
  • for media query min-width: 59.6875em: the change here is only in h5 tag. The result is: https://ibb.co/HCBFLLM ;
  • for media query min-width: 55em: I had to change the font-size to h6 HTML too like in the media query 68.75em because the values of h4 and h6 don't give us again a big range to we can find suitable value for h5 tag. The result is: https://ibb.co/dDXh2KC ;
  • for media query min-width: 46.25em: the change for this query is in h5 HTML tag only and this is the result https://ibb.co/LNJ0YSv .

Trac ticket: https://core.trac.wordpress.org/ticket/52028

@viralsampat
2 years ago

I have changed the tag font size to 20 pixels for desktop and also made respective changes for responsive devices. Please let me know if you have any query about the same.

This ticket was mentioned in PR #2496 on WordPress/wordpress-develop by AlanSyue.


2 years ago
#5

# How
I increased 1px to all the H5 font sizes on theme Twenty Fifteen. I think it got a little bit different between the H5 and H6 font sizes.

# Testing Instructions
## Active the Twenty Fifteen theme
https://i0.wp.com/user-images.githubusercontent.com/33183531/161375675-348a26bd-0379-488a-be94-88692dcda180.png

## Open the editor and use H4 (20px), H5 (18px), H6 (17px)
https://i0.wp.com/user-images.githubusercontent.com/33183531/161375799-9f6de6e1-f4a2-420b-8a78-673840a14621.png

Trac ticket: https://core.trac.wordpress.org/ticket/52028

#6 @stephencamilo
2 years ago

  • Keywords close has-screenshots added
  • Resolution set to worksforme
  • Status changed from new to closed

#7 @sabernhardt
2 years ago

  • Keywords close removed
  • Resolution worksforme deleted
  • Status changed from closed to reopened

This ticket was mentioned in Slack in #core by sergey. View the logs.


2 years ago

#9 @costdev
2 years ago

@poena and @sabernhardt, will either/both of you have time to review the current patches before RC1 to see what you think is the best approach, or should we move this to a different milestone such as 6.1 or Future Release?

#10 @costdev
2 years ago

  • Keywords dev-feedback added

#11 @sabernhardt
2 years ago

I think PR 2496 is the best approach, though I'm not certain yet. I'll create screenshots to compare against the current styling in each situation: in the editor, on the frontend (at 320, 768, 920, 1000, 1160 and 1280 pixels wide), with IE11, and in list blocks.

#12 @alansyue
2 years ago

@sabernhardt
I have adjusted the PR according to your suggestion, please help to check when you are free, thanks.

Last edited 2 years ago by alansyue (previous) (diff)

#13 @costdev
2 years ago

  • Milestone changed from 6.0 to 6.1

As 6.0 RC1 starts very soon, I'm moving this to the 6.1 milestone.

sabernhardt commented on PR #2496:


2 years ago
#14

Below are screenshots with the patch. If you would like to compare, I also made a static collection of 'before' screenshots and an animated GIF that alternates between both every five seconds.

Each screenshot includes H4, H5 and H6 examples with a paragraph between the headings. The headings have a second line to show the line-height apart from the margins.

The fourth column displays these headings with the same styles at any screen size in the block editor because .editor-block-list__block was removed in WordPress 5.4. If that should be fixed in Twenty Fifteen, let's address it on a _separate ticket_ because it involves all heading levels.

https://i0.wp.com/user-images.githubusercontent.com/17100257/170765791-0280d53d-891d-4eff-a647-1df35eb120cb.png

And I said I would create Internet Explorer screenshots, though the IE stylesheet is for versions 6-8:

https://i0.wp.com/user-images.githubusercontent.com/17100257/170765812-71eabc64-4359-4e7d-a060-a9e5aaff37c1.png

AlanSyue commented on PR #2496:


2 years ago
#15

Thanks, this is really helpful. Is there anything else I can help with?

#16 @sabernhardt
2 years ago

PR 2496 works for me. @poena, what do you think?

Last edited 2 years ago by sabernhardt (previous) (diff)

@pratiweb
2 years ago

Increase H5 font size to 18px for all content.

@russel07
2 years ago

I have updated the font size and margin properties in several places.

#17 @russel07
2 years ago

I have found in some cases the font size and margin are used the same for h5 and h6 in style.css
For example

.comment-content h5, .comment-content h6 {

font-size: 15 px

}

Both h5 and h6 have used the same style. I have updated in all places and working as expected.

This ticket was mentioned in Slack in #core by sabernhardt. View the logs.


2 years ago

#19 @sabernhardt
2 years ago

  • Keywords needs-design-feedback added
  • Type changed from defect (bug) to enhancement

As @joyously noted, the original design matched H5 and H6 styles, so changing it is more of an enhancement.

I would like feedback about the design of PR 2496 to determine if any adjustments ought to be made to that or if it's ready to commit.

This ticket was mentioned in Slack in #core by desrosj. View the logs.


22 months ago

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


22 months ago

#22 @audrasjb
22 months ago

  • Keywords commit added; dev-feedback needs-design-feedback removed

As per today's bugscrub, let's mark PR2496 as ready for commit.

Thanks to @jeffpaul for review/advices.

Last edited 22 months ago by audrasjb (previous) (diff)

#23 @audrasjb
22 months ago

  • Owner set to audrasjb
  • Status changed from reopened to accepted

#24 @audrasjb
22 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 54109:

Twenty Fifteen: Increase the font size used for h5 headings.

This changeset ensures h5 and h5 heading levels don't use the same font size.

Follow-up to [29892].

Props poena, akabarikalpesh, hellofromTonya, afrid1719, aezazshekh, sabernhardt, anitanenova, viralsampat, alansyue, pratiweb, russel07.
Fixes #52028.

#25 @audrasjb
22 months ago

Added missing @jeffpaul props manually via Make/Core.

Note: See TracTickets for help on using tickets.