Make WordPress Core

Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#45912 closed enhancement (fixed)

Twenty Nineteen: Horizontal rule is very narrow

Reported by: laurelfulford's profile laurelfulford Owned by: ianbelanger's profile 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 5 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 5 years ago.
45912.1.diff (3.2 KB) - added by leprincenoir 4 years ago.
45912-1-diff.png (79.8 KB) - added by samful 4 years ago.
45912-above-768px.png (9.7 KB) - added by ianbelanger 4 years ago.
Above 768px
45912-below-768px.png (10.4 KB) - added by ianbelanger 4 years ago.
Below 768px
45912.2.diff (2.7 KB) - added by leprincenoir 4 years ago.
Normally, she's the one

Download all attachments as: .zip

Change History (25)

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

  • Description modified (diff)

#2 @nielslange
5 years ago

Howdy @laurelfulford 👋

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

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

Thanks, @laurelfulford.

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

@nielslange
5 years ago

#5 @nielslange
5 years ago

The patch is ready for testing. 😀

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

#6 @nielslange
5 years ago

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

#7 @kjellr
5 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
4 years ago

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

#9 @samful
4 years 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 years 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
4 years ago

#11 @samful
4 years ago

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

@samful
4 years ago

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

Above 768px

@ianbelanger
4 years ago

Below 768px

@leprincenoir
4 years ago

Normally, she's the one

#13 @ianbelanger
4 years 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
4 years 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
4 years 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
4 years 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
4 years 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
4 years 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.