WordPress.org

Make WordPress Core

Opened 21 months ago

Closed 3 months ago

Last modified 2 months 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 21 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 20 months ago.
45912.1.diff (3.2 KB) - added by leprincenoir 3 months ago.
45912-1-diff.png (79.8 KB) - added by samful 3 months ago.
45912-above-768px.png (9.7 KB) - added by ianbelanger 3 months ago.
Above 768px
45912-below-768px.png (10.4 KB) - added by ianbelanger 3 months ago.
Below 768px
45912.2.diff (2.7 KB) - added by leprincenoir 3 months ago.
Normally, she's the one

Download all attachments as: .zip

Change History (25)

@laurelfulford
21 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
21 months ago

  • Description modified (diff)

#2 @nielslange
21 months ago

Howdy @laurelfulford 👋

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

#3 @laurelfulford
20 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
20 months ago

Thanks, @laurelfulford.

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

@nielslange
20 months ago

#5 @nielslange
20 months ago

The patch is ready for testing. 😀

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

#6 @nielslange
20 months ago

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

#7 @kjellr
20 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
7 months ago

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

#9 @samful
4 months 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
3 months 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.

#11 @samful
3 months ago

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

@samful
3 months ago

#12 @ianbelanger
3 months 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 months ago

Above 768px

@ianbelanger
3 months ago

Below 768px

@leprincenoir
3 months ago

Normally, she's the one

#13 @ianbelanger
3 months 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 months 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 months 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 months 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.

#17 @desrosj
2 months ago

In 48514:

Bundled Themes: Rebuild Twenty Nineteen’s RTL stylesheet.

This adds the changes from [48073] to the style-rtl.css file.

See #45912.

#18 @SergeyBiryukov
2 months ago

In 48605:

Bundled Themes: Rebuild Twenty Nineteen’s RTL stylesheet.

This adds the changes from [48073] to the style-rtl.css file.

Previously committed in [48514], accidentally reverted in [48602].

See #45912, #49843.

Note: See TracTickets for help on using tickets.