Make WordPress Core

Opened 2 years ago

Closed 3 months ago

Last modified 3 months ago

#60770 closed enhancement (fixed)

TimeZone select box compatibility with RTL directions

Reported by: farhad0's profile farhad0 Owned by: audrasjb's profile audrasjb
Milestone: 7.0 Priority: normal
Severity: normal Version: 6.5
Component: I18N Keywords: has-patch has-screenshots has-test-info commit
Focuses: tests, rtl, administration Cc:

Description

This patch fixes the time zone select box compatibility with RTL directions It sets auto direction which makes the options value compatible with both RTL and LTR directions

Attachments (3)

timezone-select-box-rtl-compatibility.jpg (116.6 KB) - added by farhad0 2 years ago.
timezone-select-box-rtl-compatibility-before-after
rtl_before.png (44.0 KB) - added by drysand 2 years ago.
rtl_after.png (43.1 KB) - added by drysand 2 years ago.

Download all attachments as: .zip

Change History (21)

This ticket was mentioned in PR #6268 on WordPress/wordpress-develop by @farhad0.


2 years ago
#1

This patch fixes the time zone select box compatibility with RTL directions It sets auto direction which makes the options value compatible with both RTL and LTR directions

Trac ticket: https://core.trac.wordpress.org/ticket/60770

---

#2 @swissspidy
2 years ago

  • Keywords needs-testing-info needs-screenshots added

@farhad0 Could you perhaps add before/after screenshots so that others looking at this ticket can better understand the problem you're trying to solve? Thanks!

@farhad0
2 years ago

timezone-select-box-rtl-compatibility-before-after

#3 @farhad0
2 years ago

  • Keywords has-screenshots added; needs-screenshots removed

@swissspidy
Hi,
Here you are
All RTL items are aligned and directed to right and other LTR items are in left

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

@drysand
2 years ago

@drysand
2 years ago

#4 @drysand
2 years ago

i tested the patch and it does indeed align english text LTR and RTL text RTL, however it seems to align all the optiongroup headers LTR no matter the language

sorry about the separate images, im still getting used to trac

#5 @sabernhardt
13 months ago

#63184 was marked as a duplicate.

#6 @sabernhardt
13 months ago

  • Component changed from Administration to I18N
  • Milestone changed from Awaiting Review to 6.9
  • Owner set to audrasjb
  • Status changed from new to assigned

In RTL (fa-IR):

  • PR 6268 aligns every optgroup plus 'UTC' and all UTC offset options to the left, and the other options align right.
  • PR 8600 aligns the options for 'UTC' and the UTC offsets to the left, but all optgroup labels and most options align to the right.

I would like to consider using the same alignment for the 'UTC' optgroup and its option (I prefer both aligned left in Firefox but both on the right seems better in Chrome).

#7 @farhad0
13 months ago

@sabernhardt
Hi,
The Chrome detects the direction of the text and align it automatically,
This is a way which many modern applications use and this works on Chrome and may be some other browsers

#8 @SirLouen
12 months ago

  • Keywords has-test-info dev-feedback added; needs-testing-info removed

Updating keywords to reflect current status.

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


8 months ago

#10 @khoipro
8 months ago

  • Focuses tests added
  • Keywords needs-testing added

I agreed with @farhad0 so we should have test if the modern browsers already support alignment without any extra code.

#11 @olmostblue
7 months ago

Hi, I have tested this fix with Chrome, Safari, Comet and Firefox.
Everything works fine on Firefox, all the rtl text are aligned to right and latin text on the left like in the After screenshot.
With Safari and Chromium based browser all the texts in the select options are aligned to the right.

Last edited 7 months ago by olmostblue (previous) (diff)

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


7 months ago

#13 @welcher
7 months ago

  • Milestone changed from 6.9 to 7.0

6.9 Beta 1 is tomorrow and this doesn't look to be ready so I'm going to move it to the 7.0 milestone.

#14 @farhad0
7 months ago

@welcher

Hi,
This issue tested and it's ready
Please review that again
Thank you

#15 @sajib1223
4 months ago

  • Keywords needs-testing removed

Test Report

Description

This report validates whether the indicated patch works as expected.

Patch tested: https://github.com/WordPress/wordpress-develop/pull/6268

Environment

  • WordPress: 6.9
  • PHP: 8.2.29
  • Server: nginx/1.27.5
  • Database: mysqli (Server: 8.0.44 / Client: mysqlnd 8.2.29)
  • Browser: Firefox 147.0
  • OS: Windows 10/11
  • Theme: Twenty Twenty-Four 1.0
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.1

Actual Results

  1. ✅ Issue resolved with patch.

Additional Notes

  • Also Tested on Chrome 143.0.0.0, works fine.
  • Also Tested on WordPress 6.6-alpha-57778-src (PR version), works fine.
  • Also Tested on WordPress 7.0-alpha-61215-src, works fine.

Supplemental Artifacts

Screenshot showing correct option text alignment:

https://files.catbox.moe/i6vpik.png

#16 @audrasjb
3 months ago

  • Keywords commit added; dev-feedback removed
  • Status changed from assigned to accepted

I tested the PR and it works fine on my side.
I think using dir="auto" is a good first step here, as it delegates the alignement to the user agent. Let's commit this.

#17 @audrasjb
3 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 61691:

I18N: Add dir="auto" to Timezone dropdown options.

This changeset adds dir="auto" to the Timezones dropdown located in Settings > General. With this attribute, the option alignment will be delegated to the user agent.

@see https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Global_attributes/dir.

Props farhad0, swissspidy, drysand, sabernhardt, khoipro, olmostblue, sajib1223.
Fixes #60770.

#18 @audrasjb
3 months ago

In 61692:

Coding Standards: Remove an unwanted space after [61691].

Follow-up to [61691].

Unprops audrasjb.
See #60770.

Note: See TracTickets for help on using tickets.