Make WordPress Core

Opened 3 years ago

Closed 2 months ago

#58722 closed defect (bug) (fixed)

Fix RTL display in Optional grouping on Permalinks Settings page

Reported by: sabernhardt's profile sabernhardt Owned by: joedolson's profile joedolson
Milestone: 7.0 Priority: normal
Severity: normal Version:
Component: Permalinks Keywords: has-patch has-test-info dev-feedback commit
Focuses: rtl, multisite Cc:

Description

splitting from #47755 (props rachid84, johnbillion, SergeyBiryukov, costdev, joedolson, afercia, ryokuhi, oglekler)

For the Category/Tag Base fields, subdirectory network installations can show a /blog prefix before the input field. In RTL languages, the slash is on the wrong side. Also, the example URL can wrap awkwardly to the next line. In any language, the text is stacked above the field with little space between them on smaller screens.

  1. Create a multisite network with subdirectories.
  2. Open the main site.
  3. Visit Settings -> Permalinks.
  4. Scroll down to the Optional section.

Change History (17)

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


3 years ago
#1

  • Keywords has-patch added

In the main site of subdirectory network installations, the blog prefix does not display correctly in RTL.

LTR (at 820 pixels wide and at 600):

https://i0.wp.com/user-images.githubusercontent.com/17100257/177835146-9eec7d5c-8cf5-424e-bc22-77f89cbedb7b.png

https://i0.wp.com/user-images.githubusercontent.com/17100257/177835149-89e628d9-b00b-49c1-b7ee-3e57697fa8e3.png

In RTL, the slash displays on the wrong side of "blog" and the prefix appears on the wrong side of the input field. Plus, the example URL can wrap awkwardly to the next line.

https://i0.wp.com/user-images.githubusercontent.com/17100257/177835151-82d8d5f4-1d0d-4ea1-9aa3-6766f2b9cf55.png

https://i0.wp.com/user-images.githubusercontent.com/17100257/177835150-4036b888-f931-41d4-abdf-0c365d0a0cf0.png

This wraps the prefix and field in a span with the code class and adds CSS to keep the prefix next to the field on smaller screens. In the paragraph, code elements are set to inline-block to keep them on one line (or wrap better).

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

#3 @costdev
3 years ago

  • Keywords has-testing-info added

#4 @sabernhardt
3 years ago

  • Milestone changed from 6.3 to Future Release

#5 @wordpressdotorg
12 months ago

  • Keywords has-test-info added; has-testing-info removed

#6 follow-up: @SirLouen
12 months ago

  • Keywords changes-requested added; needs-testing removed

Test Report UPDATED

Description

✅ This report can validate that the indicated patch works as expected.

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

Environment

  • WordPress: 6.9-alpha-60093-src
  • PHP: 8.2.28
  • Server: nginx/1.27.5
  • Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
  • Browser: Chrome 136.0.0.0
  • OS: Windows 10/11
  • Theme: Twenty Twenty-Five 1.2
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.0

Actual Results

  1. ✅ Issue resolved with patch

Additional Notes

The العربية version at 600px doesn't place the blog/ on the left, but at the top. I don't feel it's completely wrong, but maybe worth being re-checked if this is the expected behavior.

Given how short these inputs can become, placing this at the top for 600px, feels right, so either English version could go on top and leave العربية as-is, or sort العربية to the left.

UPDATE:

As @sabernhardt pointed out, I completely forgot to update the CSS build; hence it was not displaying as expected the inline-block style. Now both styles, English and العربية are being displayed the same.

Still The original version in English shows the /blog on top (for 600px) and now it displays it on the left, maybe, as I say, leaving less space for the input in shorter screens.

Supplemental Artifacts

✅English version w/ Patch @ 600px
https://i.imgur.com/hdfGUBf.png

✅ العربية version w/ Patch @ 600px
https://i.imgur.com/8NvHKj0.png

Last edited 12 months ago by SirLouen (previous) (diff)

#7 in reply to: ↑ 6 ; follow-up: @sabernhardt
12 months ago

I think your test did not update the forms-rtl(.min).css because the PR includes flex styles to put the prefix on the left. (I use npm run dev to compile RTL stylesheets.)

However, when the RTL language has the /blog prefix on the left, Firefox splits the slash on a separate line. Adding white-space: nowrap (or similar) could fix that, but removing the permalink-structure-has-blog-prefix class and its special CSS would be simpler. Then the prefix would stay above the input field at 782px and smaller.

#8 in reply to: ↑ 7 @SirLouen
12 months ago

  • Keywords dev-feedback added; changes-requested removed

Replying to sabernhardt:

I think your test did not update the forms-rtl(.min).css because the PR includes flex styles to put the prefix on the left. (I use npm run dev to compile RTL stylesheets.)

Correct, I completely forgot to update the build files. Report updated.

@sabernhardt commented on PR #2955:


12 months ago
#9

I added nowrap to avoid breaking the prefix on two lines in Firefox. Also, the condition did not need to use the empty() function when $blog_prefix can only be either /blog or an empty string.

Having the prefix on the left can make the input field a bit smaller. However, it should be enough, even on 320-pixel screens (where the field is about 248 pixels wide, and can show 24 characters before scrolling).

https://github.com/user-attachments/assets/11aa03a0-54e2-46e2-adeb-744c39a7d694

https://github.com/user-attachments/assets/51267902-cb21-46b8-badf-169c4b26679c

#10 @sabernhardt
12 months ago

PR 2955 still uses flex to keep the prefix on the left because I think that makes more sense with a category and/or tag base in each field.

#11 @sabernhardt
5 months ago

  • Milestone changed from Future Release to 7.0

@geminorum used the no-break class for a similar fix on #64381, and I updated PR 2955 to use that instead of a new .permalink-structure-has-blog-prefix code ruleset.

I'm also setting the milestone to try resolving both tickets in the same release.

Last edited 5 months ago by sabernhardt (previous) (diff)

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


2 months ago

#13 @audrasjb
2 months ago

  • Keywords needs-testing added

As per today's bug scrub: The patch looks good at a glance. I think it just needs some more testing and we can ship it. I refreshed it against trunk.

#14 @joedolson
2 months ago

  • Owner set to joedolson
  • Status changed from new to accepted

#15 @huzaifaalmesbah
2 months ago

Patch Testing Report

Patch Tested: https://github.com/WordPress/wordpress-develop/pull/2955

Environment

  • WordPress: 7.0-beta2-61752-src
  • PHP: 8.2.29
  • Server: nginx/1.29.5
  • Database: mysqli (Server: 8.4.8 / Client: mysqlnd 8.2.29)
  • Browser: Chrome 145.0.0.0
  • OS: macOS
  • Theme: Twenty Sixteen 3.7
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.1

Steps taken

  1. Created a Multisite Network using subdirectories.
  2. Logged in to the main site dashboard.
  3. Navigated to Settings → Permalinks.
  4. Scrolled to the Optional section containing Category Base and Tag Base fields.
  5. Tested the layout in LTR (English) and RTL (Arabic) languages before applying the patch.
  6. Applied the patch from PR 2955
  7. Re-tested the same screen after applying the patch.
  1. ✅ Patch is solving the problem

Screenshots/Screencast with results

Arabic (RTL)

Before Apply Patch After Apply Patch ✅
https://i.ibb.co/q3TNGmGb/Huzaifa-20260304152054.png https://i.ibb.co/nSRbbBg/Huzaifa-20260304152301.png

English

Before Apply Patch After Apply Patch ✅
https://i.ibb.co/fdHdM8JT/Huzaifa-20260304152547.png https://i.ibb.co/fY7Fp6HT/Huzaifa-20260304152426.png

#16 @joedolson
2 months ago

  • Keywords commit added; needs-testing removed

This looks good. Marking for commit.

#17 @joedolson
2 months ago

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

In 61826:

Permalinks: Improve display for permalinks slug inputs.

Set the /blog prefix to place the / correctly in RTL languages. Adjust the layout to prevent awkward wrapping in all languages on smaller viewports.

Props sabernhardt, rachid84, johnbillion, SergeyBiryukov, costdev, joedolson, afercia, ryokuhi, oglekler, sirlouen, audrasjb, huzaifaalmesbah.
Fixes #58722. See #47755.

Note: See TracTickets for help on using tickets.