WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 12 months ago

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

Download all attachments as: .zip

Change History (25)

@laurelfulford
2 years 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
2 years ago

  • Description modified (diff)

#2 @nielslange
2 years ago

Howdy @laurelfulford 👋

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

#3 @laurelfulford
2 years 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
2 years ago

Thanks, @laurelfulford.

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

@nielslange
2 years ago

#5 @nielslange
2 years ago

The patch is ready for testing. 😀

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

#6 @nielslange
2 years ago

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

#7 @kjellr
2 years 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
16 months ago

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

#9 @samful
13 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
12 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
12 months ago

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

@samful
12 months ago

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

Above 768px

@ianbelanger
12 months ago

Below 768px

@leprincenoir
12 months ago

Normally, she's the one

#13 @ianbelanger
12 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
12 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
12 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
12 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
11 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
11 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.