-
Notifications
You must be signed in to change notification settings - Fork 163
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.
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
[Text] Fix text not wrapping #2742
base: main
Are you sure you want to change the base?
Conversation
This reverts commit 20130c2.
@@ -21,7 +21,7 @@ function onChangeUncontrolled(_e: InteractionEvent, isChecked: boolean) { | |||
const BasicCheckbox: React.FunctionComponent = () => { | |||
return ( | |||
<View> | |||
<Checkbox label="Unchecked checkbox (undefined)" onChange={onChangeUncontrolled} /> | |||
<Checkbox label="Unchecked checkbox (undefined) aidasdijk ahsdihsaidhas jasdhasid" onChange={onChangeUncontrolled} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should leave the previous text only.
As we have already done our testing, this is not required for tester.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or may be add a bigger valid(meaningful) string?
@@ -96,6 +96,7 @@ export const Text = compressible<TextProps, TextTokens>((props: TextProps, useTo | |||
// now build the text style from tokens that can be shared between different Text instances | |||
const [tokenStyle] = cache( | |||
() => ({ | |||
flexShrink: 1, // Fixes bug in RN where text does not wrap correctly. RN bug #1438 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am wondering why Checkbox and Switch snapshot didn't change as they have used the Text same as Radio.
If we have redundant flexShrink:1 on Checkbox and Switch , please remove that from there and let's have it on Text only.
@@ -138,7 +139,8 @@ export const Text = compressible<TextProps, TextTokens>((props: TextProps, useTo | |||
|
|||
// return a continuation function that allows this text to be compressed | |||
return (extra: TextProps, children: React.ReactNode) => { | |||
const mergedProps = { | |||
const mergedProps: TextProps = { | |||
accessibilityRole: 'text', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is good addition. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missed this, same fix was made for Switch for accessibility issue here: #2644
Platforms Impacted
Description of changes
This PR fixes that.
(FlexShrink property was added to fix this, it is till overridable through props)
Verification
Android developer option - Layout Bounds has been enabled for better visualization.
Pull request checklist
This PR has considered (when applicable):