WordPress.org

Make WordPress Core

Opened 18 months ago

Closed 3 days ago

#45912 closed enhancement (fixed)

Twenty Nineteen: Horizontal rule is very narrow

Reported by: laurelfulford Owned by: ianbelanger
Milestone: 5.5 Priority: normal
Severity: normal Version: 5.0
Component: Bundled Theme Keywords: has-screenshots good-first-bug has-patch
Focuses: css Cc:

Description (last modified by laurelfulford)

Originally reported by @joyously in the Twenty Nineteen GitHub repo:

The <hr> tag outputs a line that is very narrow (max-width: 2.25em), and seems like it would be an unexpected style in legacy content.

It's matches the current default style for the separator block, but it's possible to change that style to be full-width, or centred dots; for the <hr> tag it can't be changed.

For the purpose of preserving the expected visual appearance in legacy content, let's switch its appearance with that of the wide separator block style.

Original ticket here: https://github.com/WordPress/twentynineteen/issues/83

Attachments (7)

83-horizontal-rule.png (5.9 KB) - added by laurelfulford 18 months ago.
Twenty Nineteen three separator block variants. hr tags currently look like the first one, but should be updated to look like the second one.
45912.diff (23.4 KB) - added by nielslange 18 months ago.
45912.1.diff (3.2 KB) - added by leprincenoir 3 weeks ago.
45912-1-diff.png (79.8 KB) - added by samful 3 weeks ago.
45912-above-768px.png (9.7 KB) - added by ianbelanger 3 weeks ago.
Above 768px
45912-below-768px.png (10.4 KB) - added by ianbelanger 3 weeks ago.
Below 768px
45912.2.diff (2.7 KB) - added by leprincenoir 3 weeks ago.
Normally, she's the one

Download all attachments as: .zip

Change History (23)

@laurelfulford
18 months ago

Twenty Nineteen three separator block variants. hr tags currently look like the first one, but should be updated to look like the second one.

#1 @laurelfulford
18 months ago

  • Description modified (diff)

#2 @nielslange
18 months ago

Howdy @laurelfulford 👋

I’ld love to provide a patch for this issue the next days! 😉

#3 @laurelfulford
18 months ago

  • Milestone changed from Awaiting Review to Future Release
  • Owner set to nielslange
  • Status changed from new to assigned

Thanks @nielslange!

I'll assign this to you for now, so it's known that it's "taken" :)

#4 @nielslange
18 months ago

Thanks, @laurelfulford.

Last edited 18 months ago by nielslange (previous) (diff)

@nielslange
18 months ago

#5 @nielslange
18 months ago

The patch is ready for testing. 😀

Last edited 18 months ago by nielslange (previous) (diff)

#6 @nielslange
18 months ago

@laurelfulford, @allancole or @kjellr: Anyone up for a quick test? 😛

#7 @kjellr
18 months ago

  • Keywords has-patch added; needs-patch removed

This works for me!

Before:
https://cldup.com/FQCaVSip_t-3000x3000.png

After:
https://cldup.com/1sT08PEfSO-3000x3000.png

Thanks, @nielslange!

#8 @ianbelanger
5 months ago

  • Focuses css added
  • Keywords commit added
  • Milestone changed from Future Release to 5.5
  • Version set to 5.0

#9 @samful
6 weeks ago

16 months later, I tested the patch today and apart from some line ending issues my end, this patch still seems to work well. @kjellr 's screenshots are still accurate.

#10 @ianbelanger
4 weeks ago

  • Keywords needs-refresh added; commit removed

The most recent patch now fails to apply, so I am removing commit and marking with needs-refresh.

@leprincenoir
3 weeks ago

#11 @samful
3 weeks ago

@leprincenoir's patch works for me, screen-shot below:

@samful
3 weeks ago

#12 @ianbelanger
3 weeks ago

Thanks for you patch @leprincenoir and for testing @samful. The patch works good above 768px, but below 768px the <hr> reverts back to the narrow width. Screenshots coming.

I believe it is due to the use of the postContentMaxWidth() function, which only includes media queries for tablet and desktop sizes.

@ianbelanger
3 weeks ago

Above 768px

@ianbelanger
3 weeks ago

Below 768px

@leprincenoir
3 weeks ago

Normally, she's the one

#13 @ianbelanger
3 weeks ago

  • Keywords commit added; needs-refresh removed
  • Owner changed from nielslange to ianbelanger
  • Status changed from assigned to reviewing

Patch looks good, thanks for the quick update @leprincenoir. Reviewing for commit now.

#14 @ianbelanger
3 weeks ago

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

In 48073:

Bundled Themes: Twenty Nineteen horizontal rule is very narrow.

Fixes the issue by adding specific styles for the <hr> tag.

Props laurelfulford, nielslange, kjellr, samful, leprincenoir.
Fixes #45912.

#15 @ianbelanger
3 weeks ago

  • Keywords fixed-major added; commit removed
  • Milestone changed from 5.5 to 5.4.3
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for backport.

#16 @desrosj
3 days ago

  • Keywords fixed-major removed
  • Milestone changed from 5.4.3 to 5.5
  • Resolution set to fixed
  • Status changed from reopened to closed

With 5.5 just under 5 weeks away, there is no 5.4.3 planned. Moving back to the 5.5 milestone.

Note: See TracTickets for help on using tickets.