Skip to content

Conversation

@mcoker
Copy link
Contributor

@mcoker mcoker commented Sep 14, 2023

fixes #9630

This makes the isCode variation retain dir="ltr" on the toggle text input and expanded content box.

Verified with an RTL speaker that it looks as expected, as well as these examples all look correct and copy/paste the values correctly.

Inline compact

Screenshot 2023-09-14 at 5 45 47 PM

Code

Screenshot 2023-09-14 at 5 51 33 PM

Regular/expanded

Screenshot 2023-09-14 at 5 52 24 PM

Array/expanded

Screenshot 2023-09-14 at 5 54 58 PM

@patternfly-build
Copy link
Contributor

patternfly-build commented Sep 14, 2023

Copy link
Contributor

@thatblindgeye thatblindgeye left a comment

Choose a reason for hiding this comment

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

As part of this, we should update the Toggle icon to mirror in RTL. When collapsed, the caret icon should be facing towards the text input.

LTR:

image

RTL:
image

value={this.state.text as string | number}
id={`text-input-${id}`}
aria-label={textAriaLabel}
dir={isCode && 'ltr'}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: a lot of places we'll spread an object for a case like this:

Suggested change
dir={isCode && 'ltr'}
{...(isCode && { dir: 'ltr' })}

Both ways result in the same result, though, with the dir attribute not being applied at all if isCode isn't true, so not a blocker.

@thatblindgeye
Copy link
Contributor

Just needs snapshots updated but otherwise looks good

Copy link
Contributor

@tlabaj tlabaj left a comment

Choose a reason for hiding this comment

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

lgtm

@tlabaj tlabaj merged commit 584cf2e into patternfly:main Sep 19, 2023
@patternfly-build
Copy link
Contributor

Your changes have been released in:

  • @patternfly/react-code-editor@5.1.1-prerelease.13
  • @patternfly/react-core@5.1.1-prerelease.13
  • @patternfly/react-docs@6.1.1-prerelease.14
  • @patternfly/react-integration@5.1.1-prerelease.7
  • demo-app-ts@5.1.1-prerelease.12
  • @patternfly/react-table@5.1.1-prerelease.13

Thanks for your contribution! 🎉

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.

Clipboard copy - RTL support

5 participants