-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Upgrade to react-native-web v0.19.9 #24482
Conversation
Hey! I see that you made changes to our Form component. Make sure to update the docs in FORMS.md accordingly. Cheers! |
@aimane-chnaif Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
🚀 Deployed to production by https://github.com/puneetlath in version: 1.3.96-15 🚀
|
🚀 Deployed to staging by https://github.com/mountiny in version: 1.3.98-0 🚀
|
🚀 Deployed to production by https://github.com/roryabraham in version: 1.3.98-5 🚀
|
var isEventTargetScrollable = scrollLength > clientLength; | ||
var delta = this.props.horizontal ? ev.deltaX || ev.wheelDeltaX : ev.deltaY || ev.wheelDeltaY; | ||
- var leftoverDelta = delta; | ||
+ var leftoverDelta = delta * 0.5; |
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.
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.
root cause here #31402 (comment)
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.
@rushatgabhane @situchan this is not something new, i just applied a patch from https://github.com/Expensify/react-native-web/pull/16/files
and was not removed until now https://github.com/Expensify/react-native-web/blob/master/packages/react-native-web/src/vendor/react-native/VirtualizedList/index.js#L815, i am pretty sure it helped with scroll issues we had.
Just a note that there was a minor regression related to this upgrade: #31101 |
@@ -14,21 +14,19 @@ function focusComposerWithDelay(textInput: TextInput | null): FocusComposerWithD | |||
return (shouldDelay = false) => { | |||
// There could be other animations running while we trigger manual focus. | |||
// This prevents focus from making those animations janky. | |||
InteractionManager.runAfterInteractions(() => { |
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.
Coming from #31210 (select an emoji with Enter
).
After we remove InteractionManager.runAfterInteractions
, the focus
will be executed in the keydown event handler, which will result in an extra newline in the composer input box.
@@ -7,6 +8,7 @@ const BaseInvertedFlatListWithRef = forwardRef((props, ref) => ( | |||
// eslint-disable-next-line react/jsx-props-no-spreading | |||
{...props} | |||
ref={ref} | |||
contentContainerStyle={styles.justifyContentEnd} |
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.
Hey there! It's been a while but still going to ask
We have passed this style to this component in the parent component https://github.com/getusha/App/blob/c97ff978ee258af698628b6a4632074dec451ddb/src/pages/home/report/ReportActionsList.js#L361
What is the reason for duplicating it here?
Coming from #31487, |
Nice job fixing patch conflict 😄 |
@@ -14,21 +14,19 @@ function focusComposerWithDelay(textInput: TextInput | null): FocusComposerWithD | |||
return (shouldDelay = false) => { | |||
// There could be other animations running while we trigger manual focus. | |||
// This prevents focus from making those animations janky. | |||
InteractionManager.runAfterInteractions(() => { |
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 also caused #27612 where after the emoji picker modal is hidden, focus is put on the composer input immediately even if we closed that modal to open another one, this resulted in a flicker because focus is lost again to the new modal.
Details
Deprecated props
nativeId
replaced withid
propfocusable
replaced withtabIndex
proppointerEvents
replaced withpointerEvents
style propertyselectable
replaced withuserSelect
style propertykeyboardType
replaced withinputMode
propthere are values which didn't have a replacement with the new prop used on native currently,
visible-password
andascii-capable
they thenkeyboardType
will still be used on native files.returnKeyType
replaced withenterKeyHint
propeditable
replaced withreadOnly
propnumberOfLines
replaced withrows
prop forTextInput
componentaccessibility*
replacedaria*
propsWe may use some
accessibility*
props in order to prevent some 3rd part libraries from breaking.for example
@testing-library/react-native
: usingaria-label
instead ofaccessibilityLabel
will cause the testing library from getting the component usingqueryByLabelText
textAlignVertical
prop replaced withverticalAlign
styleDeprecated style properties/values
transform
array value to space-separated string functions. e.g.transform: 'scale(2)'
there are limitations using the space-separated string functions with Animated components, some transforms are still using array values more info: [Animated] Cannot animate string transform properties (v0.19) necolas/react-native-web#2528
resizeMode
style replaced withresizeMode
propBreaking changes based on the usages in the codebase
position:relative
by default.InteractionManager
.More information can be found on this release note
Fixed Issues
$ #16660
PROPOSAL: #16660 (comment)
Tests
No Specific test cases
Offline tests
QA Steps
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)/** comment above it */
this
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Web
Mobile Web - Chrome
Mobile Web - Safari
Desktop
iOS
Android