Upgrade to react-native-web v0.19.9

[getusha]

Details

Deprecated props

• nativeId replaced with id prop
• focusable replaced with tabIndex prop
• pointerEvents replaced with pointerEvents style property
• selectable replaced with userSelect style property
• keyboardType replaced with inputMode prop there are values which didn't have a replacement with the new prop used on native currently, visible-password and ascii-capable they then keyboardType will still be used on native files.
• returnKeyType replaced with enterKeyHint prop
• editable replaced with readOnly prop
• numberOfLines replaced with rows prop for TextInput component
• accessibility* replaced aria* props We may use some accessibility* props in order to prevent some 3rd part libraries from breaking. for example @testing-library/react-native: using aria-label instead of accessibilityLabel will cause the testing library from getting the component using queryByLabelText
• textAlignVertical prop replaced with verticalAlign style

Deprecated 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: https://github.com/necolas/react-native-web/issues/2528
• resizeMode style replaced with resizeMode prop

Breaking changes based on the usages in the codebase

• [change] is position:relative by default.
• [change] Add task queue for InteractionManager.

More information can be found on this release note

Fixed Issues

$ https://github.com/Expensify/App/issues/16660 PROPOSAL: https://github.com/Expensify/App/issues/16660#issuecomment-1666425053

Tests

No Specific test cases

• [ ] Verify that no errors appear in the JS console

Offline tests

QA Steps

• [ ] Verify that no errors appear in the JS console

PR Author Checklist

• [x] I linked the correct issue in the ### Fixed Issues section above
• [x] I wrote clear testing steps that cover the changes made in this PR
  • [x] I added steps for local testing in the Tests section
  • [x] I added steps for the expected offline behavior in the Offline steps section
  • [x] I added steps for Staging and/or Production testing in the QA steps section
  • [x] I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
  • [x] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
  • [x] I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
• [x] I included screenshots or videos for tests on all platforms
• [x] I ran the tests on all platforms & verified they passed on:
  • [x] Android / native
  • [x] Android / Chrome
  • [x] iOS / native
  • [x] iOS / Safari
  • [x] MacOS / Chrome / Safari
  • [x] MacOS / Desktop
• [x] I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
• [x] I followed proper code patterns (see Reviewing the code)
  • [x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
  • [x] I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
  • [x] I verified that comments were added to code that is not self explanatory
  • [x] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
  • [x] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
    • [x] If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
  • [x] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
  • [x] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
  • [x] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
  • [x] I verified the JSDocs style guidelines (in STYLE.md) were followed
• [x] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
• [x] I followed the guidelines as stated in the Review Guidelines
• [x] I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
• [x] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
• [x] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
• [x] I verified that if a function's arguments changed that all usages have also been updated correctly
• [x] If a new component is created I verified that:
  • [x] A similar component doesn't exist in the codebase
  • [x] All props are defined accurately and each prop has a /** comment above it */
  • [x] The file is named correctly
  • [x] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
  • [x] The only data being stored in the state is data necessary for rendering and nothing else
  • [x] If we are not using the full Onyx data that we loaded, I've added the proper selector in order to ensure the component only re-renders when the data it is using changes
  • [x] For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
  • [x] Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
  • [x] All JSX used for rendering exists in the render method
  • [x] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
• [x] If any new file was added I verified that:
  • [x] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
• [x] If a new CSS style is added I verified that:
  • [x] A similar style doesn't already exist
  • [x] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG))
• [x] If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
• [x] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
• [x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
• [x] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
• [x] If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
• [x] If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
• [x] I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

Mobile Web - Chrome

Mobile Web - Safari

Desktop

iOS

Android

[melvin-bot[bot]]

Hey! I see that you made changes to our Form component. Make sure to update the docs in FORMS.md accordingly. Cheers!

[melvin-bot[bot]]

@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]

[aimane-chnaif]

@getusha can you please summarize changes in PR description, similar to proposal?

[getusha]

1. Replaced the fork package with upstream version.

Replaced

• keyboardType prop with inputMode We will have usages for which do not have equivalent values, for native. ascii-capable and visible-password https://github.com/Expensify/App/pull/24482/files#diff-9d6b0cc89c7cab66ac4042ac0549bfc66ddc9e7a182b6b5dbbf7f687328f10b5R760-R763
• editable prop with readOnly
• numberOfLines prop with rows
• nativeID prop with id
• focusable prop with tabIndex with values 0 and -1
• pointerEvents prop with pointerEvents style
• accessibility* prop with aria-* As of my investigation aria-* props are not supported yet in react-native-testing-library for example to query the components such as queryAllBy* methods. as a result of it using accessibilityLabel for now.
• accessibilityRole with role eslint-plugin-react-native-a11y do not recognize role only recognizes accessibilityRole. I am seeing the code and planning to raise a PR to make it support aria-* and role props. this is specifically for fixing lint errors.
• transform array values to spaced values Though we can replace non animated styles without any issue, template literal with animated values will not work. it is working only with array values. we can reach out to SWM how we can use it with spaced values from react-native-reanimated please correct me if i am wrong.
• textAlignVertical prop with verticalAlign style.

Patch for https://github.com/Expensify/react-native-web/pull/2 https://github.com/Expensify/react-native-web/pull/2 even though the code looks quite outdated. Additionally added a fix inside the patch for the following issue which was caused by scaleY(-1) https://github.com/Expensify/App/issues/16660#issuecomment-1666444603

@aimane-chnaif I hope i haven't missed a detail. thanks!

[getusha]

@aimane-chnaif raised the PR for eslint-plugin-react-native-a11y to support the role prop https://github.com/FormidableLabs/eslint-plugin-react-native-a11y/pull/150

[aimane-chnaif]

@aimane-chnaif raised the PR for eslint-plugin-react-native-a11y to support the role prop FormidableLabs/eslint-plugin-react-native-a11y#150

@roryabraham should we wait for this or patch ourselves?

[roryabraham]

I'm comfortable using patch files as long as we have created an upstream PR and actively try to get that merged 👍🏼

@getusha can you please also create an issue in https://github.com/FormidableLabs/eslint-plugin-react-native-a11y as well so that you can link the PR to it and others will be able to easily find it. That way we can more easily tell if it's resolved by another PR as well

[roryabraham]

We don't necessarily have a formal process around this yet, but our thinking is that the contributor that adds the patch is meant to be responsible for getting the change merged upstream, then upgrading the dependency to remove the patch ASAP.

[roryabraham]

@getusha I see there are a number of failing tests here + some conflicts here. I'm going to add the [WIP] prefix to the title, please remove it and ping us when this is ready for review. Thanks

[melvin-bot[bot]]

Hey, I noticed you changed some webpack configuration files. This can break production builds. Did you remember to run a production build locally to verify they still work?

[getusha]

and i raised a PR for react-native-key-command https://github.com/Expensify/react-native-key-command/pull/31 this will fix a conflict while installing packages.

@roryabraham sorry to ping you early, the reason several tests are failing ^ i already raised the PR can we get that merged? i thought it would make sense to update the version from the root instead of patching it. thanks!

[roryabraham]

Oof I forgot that Expensify/react-native-key-command isn't a fork of a 3rd-party dependency, but an original library of ours created by @azimgd. Reviewing https://github.com/Expensify/react-native-key-command/pull/31 right now...

Update: merged and published in react-native-key-command@1.0.4

[aimane-chnaif]

@getusha what is currently blocking?

[getusha]

@aimane-chnaif added patch for eslint-plugin-react-native-a11y https://github.com/Expensify/App/pull/24482/commits/916bff09978a9784ed47c334bca28bf0e88fcc48 and lint is still failing it might be related to eslint cache. I am going to remove WIP and start testing. thanks!

[getusha]

@aimane-chnaif @roryabraham It's ready for review!

[shubham1206agra]

@getusha Looks like you left a lot of accessibilityLabel prop. Can you tell me the reason for the same? I think you should make all of them aria-label for consistency.

[getusha]

@getusha Looks like you left a lot of accessibilityLabel prop. Can you tell me the reason for the same?

@shubham1206agra please check my comment here

[shubham1206agra]

@getusha Looks like you left a lot of accessibilityLabel prop. Can you tell me the reason for the same?

@shubham1206agra please check my comment here

Ok But there are a lot of accessibility props left if we ignore accessibilityLabel. Can you identify them and fix them?

[shubham1206agra]

• [ ] [change] Deprecate Touchable* components. Use Pressable.
• [ ] [change] Deprecate Button component. Use Pressable.
• [ ] [change] Deprecate StyleSheet.compose(). Use style array syntax.
• [ ] [change] Deprecate accessibility props. Use aria- and role props.
• [ ] [change] Deprecate nativeId prop. Use id prop.
• [ ] [change] Deprecate focusable props. Use tabIndex prop.
• [ ] [change] Deprecate pointerEvents prop. Use pointerEvents style.
• [ ] [change] Deprecate selectable prop. Use userSelect style.
• [ ] [change] Deprecate fontVariant style array values. Use space-separated string values.
• [ ] [change] Deprecate textAlignVertical style. Use verticalAlign style.
• [ ] [change] Deprecate transform style array values. Use space-separated string functions.
• [ ] [change] Deprecate non-standard logical styles, e.g., 'marginStart'.
• [ ] [change] Deprecate style resizeMode. Use resizeMode prop.
• [ ] [change] Deprecate style tintColor. Use tintColor prop.
• [ ] [change] Deprecate prop keyboardType. Use inputMode prop.
• [ ] [change] Deprecate prop returnKeyType. Use enterKeyHint prop.
• [ ] [change] Deprecate prop editable. Use readOnly prop.
• [ ] [change] Deprecate prop numberOfLines. Use rows prop.
• [ ] [change] Deprecate BackHandler API.

Copy this checklist and use this. It will be easier for you to verify/handle all the changes.

[shubham1206agra]

@aimane-chnaif I have doubts about one change [change] Deprecate non-standard logical styles, e.g., 'marginStart'.

Should we ignore this change as it might break cross-platform support?

[shubham1206agra]

@aimane-chnaif I have doubts about one change [change] Deprecate non-standard logical styles, e.g., 'marginStart'.

Should we ignore this change as it might break cross-platform support?

• [x] borderEndEndRadius is equivalent to borderBottomEndRadius.
• [x] borderEndStartRadius is equivalent to borderBottomStartRadius.
• [x] borderStartEndRadius is equivalent to borderTopEndRadius.
• [x] borderStartStartRadius is equivalent to borderTopStartRadius.
• [x] marginInlineStart is equivalent to marginStart.
• [x] marginInlineEnd is equivalent to marginEnd.
• [ ] marginBlockStart is equivalent to marginTop.
• [ ] marginBlockEnd is equivalent to marginBottom.
• [ ] marginBlock is equivalent to marginVertical.
• [ ] marginInline is equivalent to marginHorizontal.
• [ ] paddingInlineStart is equivalent to paddingStart.
• [ ] paddingInlineEnd is equivalent to paddingEnd.
• [ ] paddingBlockStart is equivalent to paddingTop.
• [ ] paddingBlockEnd is equivalent to paddingBottom.
• [ ] paddingBlock is equivalent to paddingVertical.
• [ ] paddingInline is equivalent to paddingHorizontal.
• [ ] inset is equivalent to top & bottom & right & left.
• [ ] insetBlock is equivalent to top & bottom.
• [ ] insetBlockEnd is equivalent to bottom.
• [ ] insetBlockStart is equivalent to top.
• [ ] insetInline is equivalent to right & left.
• [ ] insetInlineEnd is equivalent to right or left.
• [ ] insetInlineStart is equivalent to right or left

This is the checklist if we want to make this change. I think we should not make this change. What are your thoughts @aimane-chnaif @getusha ?

[aimane-chnaif]

@aimane-chnaif @roryabraham It's ready for review!

@getusha The branch conflicts and also lint failing

[getusha]

@aimane-chnaif incase you missed my comment: https://github.com/Expensify/App/pull/24482#issuecomment-1678853696 lint is passing locally.

[aimane-chnaif]

@aimane-chnaif incase you missed my comment: #24482 (comment) lint is passing locally.

lint error log:

Missing a11y props. Expected one of: accessibilityRole OR BOTH accessibilityLabel + accessibilityHint OR BOTH accessibilityActions + onAccessibilityAction react-native-a11y/has-valid-accessibility-descriptors

[getusha]

lint error log:

> eslint . --max-warnings=0 --cache --cache-location=node_modules/.cache/eslint

it's using the cache

[getusha]

here is the patch: https://github.com/Expensify/App/pull/24482/files#diff-b5cde766ba30f0666adfd5b473f588511ec53434ee1ca1af047044604cfd8d68

is there any way to reinstall the packages here?

[getusha]

@aimane-chnaif all checks have passed now.

[aimane-chnaif]

@getusha please fix conflict

[getusha]

@aimane-chnaif fixed conflicts, will fix them as they come.

[aimane-chnaif]

@getusha conflicts again

[getusha]

@aimane-chnaif Fixed.

[getusha]

@aimane-chnaif friendly bump for a review.

[kosmydel]

Hey, I've just had a quick look at this PR (because my issue is on HOLD on it).

---

I've found an issue. The ReportScreen content is centered instead of at the bottom.

Main	Staging	This PR

[getusha]

@kosmydel thanks for bringing this up here! i also identified this previously i thought i fixed it, pushed the changes now check it again.

[aimane-chnaif]

So far console warnings/errors caused by this PR:

Warnings:

• "transform" style array value is deprecated. Use space-separated string functions, e.g., "scaleX(2) rotateX(15deg)".
• "shadow*" style props are deprecated. Use "boxShadow".
• props.pointerEvents is deprecated. Use style.pointerEvents
• accessibilityLabel is deprecated. Use aria-label.
• StyleSheet.compose(a, b) is deprecated; use array syntax, i.e., [a,b].
• textAlignVertical" style is deprecated. Use "verticalAlign".
• accessibilityRole is deprecated. Use role.
• TouchableWithoutFeedback is deprecated. Please use Pressable.
• focusable is deprecated.

Errors:

• BackHandler is not supported on web and should not be used.
• Received false for a non-boolean attribute collapsable. If you want to write it to the DOM, pass a string instead: collapsable="false" or collapsable={value.toString()}.
• Provide a valid string for accessibilityLabel prop when accessible is true at PressableWithoutFeedback

[getusha]

transform array values to spaced values

Though we can replace non animated styles without any issue, template literal with animated values will not work. it is working only with array values. we can reach out to SWM how we can use it with spaced values from react-native-reanimated please correct me if i am wrong.

accessibilityLabel

aria-label not being supported by react-native-testing-library for querying components

numberOfLines (many cases)

numberOfLines is deprecated for TextInput on the release notes, so i changed it only for the text input. i believe the remaining is only applied on Text components. rows={numberOfLines}

selectable (BaseHTMLEngineProvider)

Since it was an external component i assumed it might be a custom prop which will be used, i will check again and see if we can pass stylings to the component.

[aimane-chnaif]

Web bugs so far:

• BUG1: no skeleton, search input label is not activated even though focused

https://github.com/Expensify/App/assets/96077027/3c7bebaf-48a2-4b54-9eb8-1f64375b68c0

• BUG2: crash on Assign task

https://github.com/Expensify/App/assets/96077027/ea5ba299-6acb-4553-96bf-8ebd4283e8ac

• BUG3: No space between FAB and menu

• BUG4: 0 is cut for a brief moment when open Request money page (might have same root cause as BUG1)

https://github.com/Expensify/App/assets/96077027/a3fe5c71-18f1-4c96-8fda-2c1a47b4881c

• BUG5: empty screen shows for a brief moment on New room page (might have same root cause as BUG1)

https://github.com/Expensify/App/assets/96077027/3d66c2f3-3816-4dea-85a8-f777bee3bb3f

[aimane-chnaif]

Native bugs so far:

BUG6. Console warning

BUG7: keyboard shows unnecesarily

https://github.com/Expensify/App/assets/96077027/060d9d98-6a1a-4bff-ad28-215616f70daf

BUG8: money request view position is wrong when no messages

https://github.com/Expensify/App/assets/96077027/277ff1fb-00c5-41d3-ba56-a3371a1a3b1e

BUG9: task preview style is broken, huge gap from composer

BUG10: crash when edit task description (same root cause as BUG2)

https://github.com/Expensify/App/assets/96077027/53cf6a78-640f-4295-8c34-cd5935b38897

[getusha]

BUG1: no skeleton, search input label is not activated even though focused

@aimane-chnaif thanks for the review! I Identified the bug, it causes some sort of delay across most of the components/pages i am investigating the root cause from react-native-web

[getusha]

@aimane-chnaif fixed all bugs related to web, working on the native issues 🙇

[getusha]

Though we can replace non animated styles without any issue, template literal with animated values will not work. it is working only with array values. we can reach out to SWM how we can use it with spaced values from react-native-reanimated please correct me if i am wrong.

Update: i noticed a PR Animated support for string transforms. https://github.com/necolas/react-native-web/pull/2546 I will try again following the examples.

[getusha]

BUG7: keyboard shows unnecesarily

@aimane-chnaif unable to reproduce this one.

https://github.com/Expensify/App/assets/75031127/5f3551f5-47b0-4041-ad0f-e0a603cbad14

[shubham1206agra]

@getusha Can you test this issue if this is fixed by this change? https://github.com/Expensify/App/issues/24397

[shubham1206agra]

@aimane-chnaif Can you help me merge this PR? https://github.com/necolas/react-native-web/pull/2569

[aimane-chnaif]

@aimane-chnaif Can you help me merge this PR? necolas/react-native-web#2569

you want me to review https://github.com/necolas/react-native-web/pull/2569?

[shubham1206agra]

I think you can review it. But I think you need to merge so that the warning can be solved when using a skeleton.

[getusha]

BUG6. Console warning

@aimane-chnaif i am not sure why we are giving the accessibilityRole 'text' for TextInputs and also there is no equivalent for it in the new role prop. i think we can safely remove it what do you think?

[aimane-chnaif]

BUG6. Console warning

@aimane-chnaif i am not sure why we are giving the accessibilityRole 'text' for TextInputs and also there is no equivalent for it in the new role prop. i think we can safely remove it what do you think?

I don't find any instance of setting accessibilityRole in TextInput. Can you share link?

[getusha]

I don't find any instance of setting accessibilityRole in TextInput. Can you share link?

@aimane-chnaif for example here: https://github.com/Expensify/App/blob/34d4e54055c3fee2a04e2fc11af82fe454197ff9/src/components/AmountTextInput.js#L52

[aimane-chnaif]

I don't find any instance of setting accessibilityRole in TextInput. Can you share link?

@aimane-chnaif for example here:

https://github.com/Expensify/App/blob/34d4e54055c3fee2a04e2fc11af82fe454197ff9/src/components/AmountTextInput.js#L52

you can ask why it's necessary the PR author who added it.

Meanwhile, fix all other remaining bugs