Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Home Address] displays Arabic characters from left to right #17659

Merged
merged 2 commits into from
Apr 21, 2023
+2 −1
Merged

[Home Address] displays Arabic characters from left to right #17659

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
646798d
displays Home Address from left to right
honnamkuan Apr 19, 2023
6a1b0ce
undo not needed styling changes to TextInput
honnamkuan Apr 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/components/MenuItem.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ const MenuItem = (props) => {
(props.shouldShowBasicTitle ? undefined : styles.textStrong),
(props.interactive && props.disabled ? {...styles.disabledText, ...styles.userSelectNone} : undefined),
styles.pre,
styles.ltr,
], props.titleStyle);
const descriptionVerticalMargin = props.shouldShowDescriptionOnTop ? styles.mb1 : styles.mt1;
const descriptionTextStyle = StyleUtils.combineStyles([
Expand Down Expand Up @@ -146,7 +147,7 @@ const MenuItem = (props) => {
style={titleTextStyle}
numberOfLines={1}
>
{props.title}
{StyleUtils.convertToLTR(props.title)}
</Text>
)}
{Boolean(props.description) && !props.shouldShowDescriptionOnTop && (
Expand Down
1 change: 1 addition & 0 deletions src/components/TextInput/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ class TextInput extends React.Component {
inputStyle={[
styles.baseTextInput,
styles.textInputDesktop,
styles.textAlignLeft,
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we sure we need to be applying these styles for all languages?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks a lot for the feedbacks, the styling changes to TextInput was to left align texts in Text input fields e.g. those used within AddressSearch component.

However I realised #16986 is about fixing the display of Home address field within Personal details page, so I should not be changing TextInput in the this PR associated with the issue, therefore I added a commit to undo the styling changes in TextInput.

Have done revalidation to ensure fix still works across all platforms.
image

Copy link
Member

Choose a reason for hiding this comment

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

@jasperhuangg Currently App only supports LTR mode so this might be valid. But is your question related to the languages we type or the app language (Spanish or English)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe the current issue #16986 is about getting consistent address displaying pattern in home address page, so changing TextInput might be not in scope.

Moreover, I discovered that adding textAlignLeft will cause regression of #14273 (part of the fix there involves removing textAlignLeft) , so I think we should not includes change to TextInput in this PR.

isLabeledMultiline ? styles.textInputMultiline : {},
...this.props.inputStyle,
]}
Expand Down
2 changes: 1 addition & 1 deletion src/components/TextInput/index.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const TextInput = forwardRef((props, ref) => (
// eslint-disable-next-line react/jsx-props-no-multi-spaces
autoCompleteType={props.autoCompleteType === 'new-password' ? 'password' : props.autoCompleteType}
innerRef={ref}
inputStyle={[styles.baseTextInput, ...props.inputStyle]}
inputStyle={[styles.baseTextInput, styles.textAlignLeft, ...props.inputStyle]}
/>
));

Expand Down