Skip to content

Fix RTL support across AI plugin admin UI#573

Merged
dkotter merged 9 commits into
WordPress:developfrom
t-hamano:fix/ai-home-rtl-logical-property
May 19, 2026
Merged

Fix RTL support across AI plugin admin UI#573
dkotter merged 9 commits into
WordPress:developfrom
t-hamano:fix/ai-home-rtl-logical-property

Conversation

@t-hamano
Copy link
Copy Markdown
Contributor

@t-hamano t-hamano commented May 19, 2026

Closes #483

What?

Fixes several spots in the admin UI that did not render correctly in RTL locales.

How?

  • Flip icons
  • Use logical CSS properties

Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 4.7
Used for: Auditing the codebase for RTL-sensitive code and drafting the fixes; all changes were reviewed and verified by me.

Testing Instructions

  1. Set the site language to an RTL locale (e.g. Arabic).
  2. Confirm that the elements are placed correctly. Refer to the additional comments for details.

Changelog Entry

Fixed - Correct RTL rendering of directional icons, runtime-set styles, and inline styles in the admin UI.

Open WordPress Playground Preview

t-hamano and others added 3 commits May 19, 2026 15:18
routes/ai-home/style.scss is bundled into JS via wp-build, so it does
not pass through the RTLCSS auto-flip and no -rtl version is generated.
Physical left/right properties therefore break RTL layouts. Replace
padding-left with padding-inline-start, and document the constraint in a
header comment so future styles use logical properties from the start.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
In RTL languages, "previous" points right and "next" points left, so
mirror the chevron icons with isRTL() to keep the navigation direction
consistent with reading order.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The build pipeline runs rtlcss over compiled CSS, so physical
properties in SCSS are flipped automatically. These cases bypass that
pipeline and need explicit handling:

- Directional icons/glyphs: flip chevron icons in the shared
  ImageHistoryNav and the taxonomy parent separator with isRTL().
- JS/JSX inline styles: rtlcss never sees runtime-set styles, so use
  logical properties (float: inline-end, marginInlineStart).
- PHP inline style attributes: same — switch the spinner spacing to
  margin-inline-start.
- Move the "Back to List" arrow into the translatable string so RTL
  locales can swap &larr; for &rarr;.

Also collapse the alt-text meta box echo chain into a single printf
for readability.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented May 19, 2026

Codecov Report

❌ Patch coverage is 0% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.24%. Comparing base (99d5a03) to head (5ed27d9).

Files with missing lines Patch % Lines
...iments/Alt_Text_Generation/Alt_Text_Generation.php 0.00% 17 Missing ⚠️
...udes/Experiments/Abilities_Explorer/Admin_Page.php 0.00% 2 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             develop     #573      +/-   ##
=============================================
- Coverage      72.40%   72.24%   -0.17%     
  Complexity      1162     1162              
=============================================
  Files             68       68              
  Lines           5596     5609      +13     
=============================================
  Hits            4052     4052              
- Misses          1544     1557      +13     
Flag Coverage Δ
unit 72.24% <0.00%> (-0.17%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

<div class="ability-explorer-detail">
<div class="ability-detail-header">
<a href="<?php echo esc_url( $back_url ); ?>" class="button">&larr; <?php esc_html_e( 'Back to List', 'ai' ); ?></a>
<a href="<?php echo esc_url( $back_url ); ?>" class="button"><?php echo wp_kses_post( __( '&larr; Back to List', 'ai' ) ); ?></a>
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ability Explorer

I included an HTML entity in the text. This approach is frequently used in the core. The direction of an arrow should be fixed once translators apply a translation using &rarr; instead of &larr;.

Image

t-hamano and others added 2 commits May 19, 2026 17:13
The WordPress core .spinner style applies float: left, which pushed the
spinner away from the Generate button and broke its inline alignment.
Override with float: none so the spinner stays next to the button.
Also drop the redundant aria-hidden attribute.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The button markup in add_button_to_media_modal() was built via long
string concatenation, making it hard to read and inconsistent with the
printf-based meta box markup. Switch to sprintf with positional
placeholders for parity with render_attachment_meta_box().

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment on lines -231 to +241
$button_text = empty( get_post_meta( $post->ID, '_wp_attachment_image_alt', true ) ) ? __( 'Generate', 'ai' ) : __( 'Regenerate', 'ai' );

echo '<div class="ai-alt-text-media-actions" style="margin-top: 16px;">';
echo '<button id="ai-alt-text-generate-button" class="button button-secondary" type="button" data-attachment-id="' . absint( $post->ID ) . '">' . esc_html( $button_text ) . '</button><span class="spinner" aria-hidden="true" style="margin-left: 8px;"></span><p class="description" aria-live="polite" style="margin-top: 10px; line-height: 1.3;"></p>';
echo '</div>';
$button_text =empty( get_post_meta( $post->ID, '_wp_attachment_image_alt', true ) ) ? __( 'Generate', 'ai' ) : __( 'Regenerate', 'ai' );

printf(
'<div class="ai-alt-text-media-actions" style="margin-top: 16px;">' .
'<button id="ai-alt-text-generate-button" class="button button-secondary" type="button" data-attachment-id="%1$d">%2$s</button>' .
'<span class="spinner" style="margin-inline-start: 8px; float: none;"></span>' .
'<p class="description" aria-live="polite" style="margin-top: 10px; line-height: 1.3;"></p>' .
'</div>',
absint( $post->ID ),
esc_html( $button_text )
);
Copy link
Copy Markdown
Contributor Author

@t-hamano t-hamano May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section includes three changes:

  • Changemargin-left to margin-inline-start
  • Refactor using printf for readability
  • Add float: none to the spinner
  • Add float: none to the spinner

This is a minor bug fix that is not directly related to this PR. Without it, the spinner would not be placed directly next to the button.

develop branch:

image

This branch:

image

Comment on lines -339 to +353
'html' => '<div class="ai-alt-text-media-actions"><button id="ai-alt-text-generate-button" class="button button-secondary" type="button" data-attachment-id="' . absint( $post->ID ) . '">' . esc_html( $button_text ) . '</button><span class="spinner" aria-hidden="true" style="margin-left: 8px;"></span><p class="description" aria-live="polite" style="margin-top: 6px; font-size: 12px;"></p></div>',
'html' => sprintf(
'<div class="">' .
'<button id="ai-alt-text-generate-button" class="button button-secondary" type="button" data-attachment-id="%1$d">%2$s</button>' .
'<span class="spinner" aria-hidden="true" style="margin-inline-start: 8px; float: none;"></span>' .
'<p class="description" aria-live="polite" style="margin-top: 6px; font-size: 12px;"></p>' .
'</div>',
absint( $post->ID ),
esc_html( $button_text )
),
Copy link
Copy Markdown
Contributor Author

@t-hamano t-hamano May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as #573 (comment)

Comment thread routes/ai-home/style.scss
Comment on lines -40 to +50
padding-left: 40px;
padding-inline-start: 40px;
Copy link
Copy Markdown
Contributor Author

@t-hamano t-hamano May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before After
Image Image

Copy link
Copy Markdown
Member

@simison simison May 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be best to just use WP Stylelint for this stuff so that folks don't need to think about it. Adding WP Stylelint in #594

Comment on lines -113 to +114
{ suggestion.parent + ' › ' }
{ suggestion.parent +
( isRTL() ? ' ‹ ' : ' › ' ) }
Copy link
Copy Markdown
Contributor Author

@t-hamano t-hamano May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot test this due to an error in my environment, but I believe this change is correct.

{"code":"rest_ability_invalid_method","message":"Abilities that perform updates require POST method.","data":{"status":405}}

container = document.createElement( 'span' );
container.className = 'ai-excerpt-inline-wrapper';
container.style.cssText = 'float: right;';
container.style.cssText = 'float: inline-end;';
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image

<Button
className="ai-image-history-nav__arrow"
icon={ chevronLeft }
icon={ isRTL() ? chevronRight : chevronLeft }
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image

Add a space between the assignment operator and empty() for readability
and consistency with PHP coding standards.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@t-hamano t-hamano marked this pull request as ready for review May 19, 2026 09:14
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 19, 2026

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: dkotter <dkotter@git.wordpress.org>
Co-authored-by: jeffpaul <jeffpaul@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

The sprintf refactor dropped the wrapper class, leaving an empty
class="" on the media modal div. The media modal JS locates the
spinner and status elements through the .ai-alt-text-media-actions
container and bails out when it is missing, so the click handler
was never attached and the button label never switched to
"Regenerate". Restoring the class fixes the failing E2E tests.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@dkotter dkotter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left one minor comment but otherwise looks good to me

printf(
'<div class="ai-alt-text-media-actions" style="margin-top: 16px;">' .
'<button id="ai-alt-text-generate-button" class="button button-secondary" type="button" data-attachment-id="%1$d">%2$s</button>' .
'<span class="spinner" style="margin-inline-start: 8px; float: none;"></span>' .
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We had aria-hidden="true" on the original code but not seeing that here. I do see that still on the code changes below (line 348) so wondering if that's needed here still?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note I added this back in 5bd9d63 to unblock this PR and get it in the 1.0 release.

If that was meant to be removed, let me know and we can open a follow up PR

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dkotter Sorry for the late reply. It was my simple mistake that I deleted aria-hidden="true". Thank you for restoring it.

@dkotter dkotter added this to the 1.0.0 milestone May 19, 2026
@dkotter dkotter mentioned this pull request May 19, 2026
42 tasks
@dkotter dkotter merged commit 0d63e85 into WordPress:develop May 19, 2026
18 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Provide better support for RTL languages

3 participants