WordPress.org

Make WordPress Core

Opened 8 months ago

Last modified 5 weeks ago

#46658 reviewing defect (bug)

Twenty Nineteen: Navigation menu is messy with RTL language

Reported by: manooweb Owned by: SergeyBiryukov
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: has-patch has-screenshots needs-testing
Focuses: rtl Cc:
PR Number:

Description

Hello,

For our languages switcher, we can add it in primary navigation menu with an options for adding country flag for each language.

With a rtl language like arabic, we have a strange behaviour because of the first li tag does not have the behaviour expected.

See first screenshot attached

Unlike in twenty seventeen

display: inline;

is used instead of

display: inline-block;

When I corrected to

display: inline-block;

it works well again in this case.

See second screenshot attached

Regards

Attachments (6)

messy-menu-wp.jpg (330.5 KB) - added by manooweb 8 months ago.
first screenshot
not-messy-menu-wp.jpg (333.1 KB) - added by manooweb 8 months ago.
second screenshot
correct-menu-rtl-twenty-nineteen.diff (449 bytes) - added by manooweb 8 months ago.
patch file
2019-inline-flags-mixup-ltr-english-chrome.png (16.7 KB) - added by sabernhardt 5 weeks ago.
Two RTL languages in switcher, on English page in Chrome
2019-inline-block-flags-fixed-ltr-english-chrome.png (16.6 KB) - added by sabernhardt 5 weeks ago.
Two RTL languages in primary menu list items set to display: inline-block (on English page, in Chrome)
46658.diff (1.3 KB) - added by sabernhardt 5 weeks ago.
Primary menu list items set to display: inline-block for both LTR and RTL stylesheets, plus SCSS

Download all attachments as: .zip

Change History (15)

@manooweb
8 months ago

first screenshot

@manooweb
8 months ago

second screenshot

#1 @audrasjb
8 months ago

  • Component changed from Themes to Bundled Theme
  • Version trunk deleted

#2 @SergeyBiryukov
8 months ago

  • Summary changed from [twenty nineteen]Navigation menu is messy with rtl language to Twenty Nineteen: Navigation menu is messy with RTL language

#3 @SergeyBiryukov
5 months ago

  • Milestone changed from Awaiting Review to 5.3
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

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


5 weeks ago

#5 @davidbaumwald
5 weeks ago

@SergeyBiryukov This one was brought up in the bug scrub today. Is this still on your list for 5.3?

#6 @marybaum
5 weeks ago

  • Keywords needs-testing added

@sabernhardt
5 weeks ago

Two RTL languages in switcher, on English page in Chrome

@sabernhardt
5 weeks ago

Two RTL languages in primary menu list items set to display: inline-block (on English page, in Chrome)

#7 @sabernhardt
5 weeks ago

  • Keywords needs-refresh added

I'll still test more by tomorrow to see if I can find a problem with using display: inline-block for the primary menu items, but a change to one stylesheet would need to be made for both language directions.

A second, adjacent RTL language link on a left-to-right language's page mixes up the two flags visually in Chrome, Edge and Internet Explorer (Firefox was fine). Setting the list items to inline-block in styles.css as well would take care of that.

(testing Twenty Nineteen 1.4 with Polylang 2.6.5 and WordPress 5.3 beta 3 on Windows 10)

@sabernhardt
5 weeks ago

Primary menu list items set to display: inline-block for both LTR and RTL stylesheets, plus SCSS

#8 @sabernhardt
5 weeks ago

  • Keywords needs-refresh removed

Setting to display: inline-block works for me:

  1. with or without the flags,
  2. either single-level or multi-level, and
  3. with both LTR and RTL text directions.

So the new patch includes the two stylesheets plus the related SCSS partial.

Windows browsers used:

  • Google Chrome 77.0.3865.120
  • Mozilla Firefox 69.0.3
  • Microsoft Edge 44.18362.387.0
  • Internet Explorer 11

#9 @davidbaumwald
5 weeks ago

  • Milestone changed from 5.3 to Future Release

With 5.3 RC1 releasing today, this is being moved to Future Release. If any committer feels this can be worked in quickly for 5.3 or can assume ownership in 5.4, feel free to move it back up.

Note: See TracTickets for help on using tickets.