WordPress.org

Make WordPress Core

Opened 17 months ago

Closed 17 months ago

Last modified 17 months ago

#44611 closed defect (bug) (fixed)

try Gutenberg header wraps over text below on narrow screens

Reported by: fierevere Owned by: pento
Milestone: 4.9.8 Priority: normal
Severity: normal Version: 5.1
Component: Administration Keywords: has-screenshots needs-design has-patch commit
Focuses: Cc:
PR Number:

Description

"Test the new editor today" can wrap over text below on narrow screens
https://i.imgur.com/v1hwSfy.jpg
Russian translation, screen width 1366 px

https://i.imgur.com/wqzsX30.jpg
english, narrowed screen

browser: Firefox 61

wp-4.9.8-beta1-43507

Attachments (3)

ff57.png (39.6 KB) - added by pbiron 17 months ago.
english, narrowed screen browser: Firefox 57 wp-4.9.8-beta1-43509
44611.diff (4.0 KB) - added by earnjam 17 months ago.
Screen Shot 2018-07-19 at 7.28.15 PM.png (273.9 KB) - added by earnjam 17 months ago.

Download all attachments as: .zip

Change History (25)

#1 @johnbillion
17 months ago

  • Keywords needs-patch has-screenshots added
  • Milestone changed from Awaiting Review to 4.9.8

Thanks for the report!

@pbiron
17 months ago

english, narrowed screen browser: Firefox 57 wp-4.9.8-beta1-43509

#2 @pbiron
17 months ago

appears to have something to do with the default font in FF 61, because it looks fine for me in FF 57 (and Chrome 67).

#3 @fierevere
17 months ago

https://i.imgur.com/5Us8pUw.jpg
default wp-admin font (Segoe UI bold)
Russian translation

Unable to reproduce with any screen width with english user language and default font

I ve shortened russian translation a bit, lets see after language pack update if i can reproduce the problem.
But keep in mind that some locales may offer longer translation and there is no comment for translators to limit translation string length.

Last edited 17 months ago by fierevere (previous) (diff)

#4 @fierevere
17 months ago

https://i.imgur.com/C7X4d9W.jpg

still looks bad with localized string with some (not all) widths
Screen ruler in pixels below.

Last edited 17 months ago by fierevere (previous) (diff)

#5 @fierevere
17 months ago

reproducible:
Firefox 52 ESR (Linux)
Firefox 61 (Linux and Windows)
Vivaldi 1.15.1147.52 (Linux)
by narrowing browser window (see ruler on screenshot above)

#6 @earnjam
17 months ago

This is because it's using display:grid on the containing element and then setting a fixed height of 45px on the first row item with grid-template-rows: 45px auto 100px; It's going to happen anytime the text needs to wrap, regardless of browser.

It looks like it's fine in English because the text is short enough that it hits breakpoints before the container gets small enough to require wrapping, but some of the translations are longer, so they wrap and overlap.

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


17 months ago

#8 @stino11
17 months ago

@earnjam Has the right idea. The fix should be

    grid-template-rows: auto 1fr 100px;
  • auto lets the first grid row expand to fit its content
  • 1fr lets the second grid row expand to fit the remaining available space
  • 100px fixes the height of the third grid row

#9 @earnjam
17 months ago

Other option is to wrap the <h3> and the <p> in a <div> and change that line to grid-template-rows: auto 100px though that does add some extraneous markup.

@earnjam
17 months ago

#10 @earnjam
17 months ago

44611.diff Takes the method I suggested above of wrapping in a div because it allows the buttons to remain at the bottom of the column container per the current design.

#11 @pento
17 months ago

  • Keywords needs-design has-patch added; needs-patch removed

Thanks for the patch, @earnjam!

@kjellr: How does this look to you?

#12 @fierevere
17 months ago

https://i.imgur.com/BbOnszD.jpg

after applying patch.
English looks same - misplaced overlapping buttons
The title - it does not overlap anymore, but it can be shortened to Попробуйте новый (Try new), cutting out everything that comes after редактор сегодня (editor today)

#13 @earnjam
17 months ago

@fierevere Can you do a hard refresh and test again? I think the CSS might be cached in your browser.

#14 @kjellr
17 months ago

@kjellr: How does this look to you?

Looks good to me. 👍

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


17 months ago

#16 @psykro
17 months ago

@fierevere have you been able to verify whether clearing your cache resolves the issue?

#17 follow-up: @pbiron
17 months ago

the patch does seem to address the wrapping issue.

I have 2 minor suggestions:

  • the line-height on the header is currently 2.1 (inherited from .try-gutenberg-panel)
    • I think it would look better with something around 1.3
  • when the header wraps, the paragraphs of text do not vertically align. that is not a major problem, IMHO, but would look better if they did
    • I tried to figure out how to achieve that, but was unable to

Failure to implement those suggestions should not stop us from including this in 4.9.8 RC (which goes out tomorrow).

#18 in reply to: ↑ 17 @kjellr
17 months ago

Replying to pbiron:

I have 2 minor suggestions:

  • the line-height on the header is currently 2.1 (inherited from .try-gutenberg-panel)
    • I think it would look better with something around 1.3
  • when the header wraps, the paragraphs of text do not vertically align. that is not a major problem, IMHO, but would look better if they did
    • I tried to figure out how to achieve that, but was unable to

Agreed on that first point, though I'd personally suggest 1.4 for the line height. I think it's alright if the paragraphs don't align when the header spans two (or more) lines.

In any case though, these can be addressed in a separate issue and shouldn't be blockers for this ticket.

#19 @pbiron
17 months ago

  • Keywords commit added

#21 @pento
17 months ago

  • Owner set to pento
  • Resolution set to fixed
  • Status changed from new to closed

In 43523:

Dashboard: Improve "Try Gutenberg" subheading appearance for long headings.

When translated, the callout subheadings can wrap onto a new line, which caused them to overlap the paragraph text.

Props earnjam, fierevere.
Fixes #44611.

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


17 months ago

Note: See TracTickets for help on using tickets.