WordPress.org

Make WordPress Core

Opened 17 months ago

Last modified 10 months 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:

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 17 months ago.
first screenshot
not-messy-menu-wp.jpg (333.1 KB) - added by manooweb 17 months ago.
second screenshot
correct-menu-rtl-twenty-nineteen.diff (449 bytes) - added by manooweb 17 months ago.
patch file
2019-inline-flags-mixup-ltr-english-chrome.png (16.7 KB) - added by sabernhardt 10 months 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 10 months 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 10 months 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
17 months ago

first screenshot

@manooweb
17 months ago

second screenshot

#1 @audrasjb
17 months ago

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

#2 @SergeyBiryukov
17 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
14 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.


10 months ago

#5 @davidbaumwald
10 months ago

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

#6 @marybaum
10 months ago

  • Keywords needs-testing added

@sabernhardt
10 months ago

Two RTL languages in switcher, on English page in Chrome

@sabernhardt
10 months ago

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

#7 @sabernhardt
10 months 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
10 months ago

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

#8 @sabernhardt
10 months 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
10 months 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.