React-native-web: 0.14 canary

Created on 27 Aug 2020  路  40Comments  路  Source: necolas/react-native-web

This thread will surface the main changes that I expect to go into 0.14 and give people a chance to test the pre-release.

npm install react-native-web@canary

To report an issue please comment below with a reduced test case on codesandbox and the steps to reproduce.

[change] Default flex-basis value of Views

React Native has an implementation of flexbox that does not quite follow the
W3C spec for flexbox. Previously, React Native for Web attempted to replicate
the React Native rendering by setting flexBasis to 0%. However, this created
its own problems where views could collapse down to 0px in height on the web.
This patch sets the default flexBasis back to 'auto'. This will occasionally cause
different rendering inconsistencies with React Native, which can be addressed
by making changes small to existing React Native styles. And ultimately, it is up
to Yoga to correct its flexbox implementation.

Fix #1640
Fix #1604
Fix #1264
Close #1265

[change] Add Modal implementation

A Modal implementation using CSS animations and ARIA.

The app is hidden from screen readers via aria-modal. Focus is contained
within the modal. When the Escape key is pressed, the onRequestClose
function is called on the top-most modal.

Close #1646
Fix #1020

[add] Pressable support for hover state

This patch ports the 'useHover' hook to React Native for Web, providing hover
state that is scoped to a pressable and does not bubble to ancestor pressables.
This behavior aligns with the behavior of the focus and press states.

Fix #1708

Most helpful comment

:partying_face: The PR for Modal is now merged into @next - so the canary should have that good to go!

Seems to be all set with compatibility with react-native-modal (see here) & next.js and SSR.

All 40 comments

there is problem with https://github.com/react-native-community/react-native-modal

modal could not close

I'll need every issue to be described following the normal issue template format. So a test case and steps to reproduce, as well as whether it is a regression from 0.13

Hi @necolas - do you want folks to report issues inline in this thread for viz or separately as top-level issues? Thanks a bunch - release looks great overall!

Inline please

The problem
the new Modal doesn't unmount when consumed via react-native-modal

How to reproduce
Simplified test case: https://codesandbox.io/s/dazzling-chebyshev-8d3z3?file=/src/App.js

Steps to reproduce:

  1. Show a react-native-modal on the screen, set isVisible to true
  2. Try to unmount by setting isVisible to false
  3. react-native-modal and modal children remain on the screen, even when visibility is confirmed to be false

Expected behavior
Modal should unmount. Interestingly, if we consume the vanilla Modal RNW export and mount / unmount it, things work fine - so it is definitely an interfacing issue between the new Modal implementation and the react-native-community library.

Unmounting works as expected in React Native proper (iOS and Android)

Environment (include versions). Did this work in previous versions?

  • React Native for Web (version): 0.0.0-d6e83d84a
  • React (version): 16.13.X
  • Browser: Chrome
  • This wasn't possible in prior versions of RNW 馃槃 excited that it is nearing reality!

TLDR: I think this isn't exactly related to the modal implementation, unfortunately. ):

react-native-modal isn't passing the visible flag down to the Modal.

react-native-modal sets isTransitioning on itself but never unsets it. Because of this, the close function on react-native-modal bails out early because of isTransitioning and it never actually sets the state for isVisible / passes it down to the underlying modal.

This is because react-native-animatable never sets contentRef for some reason & never can be animated.

TLDR: I think this isn't exactly related to the modal implementation, unfortunately. ):

react-native-modal isn't passing the visible flag down to the Modal.

react-native-modal sets isTransitioning on itself but never unsets it. Because of this, the close function on react-native-modal bails out early because of isTransitioning and it never actually sets the state for isVisible / passes it down to the underlying modal.

This is because react-native-animatable never sets contentRef for some reason & never can be animated.

ouch! sounds like I should investigate rolling with the raw Modal and transitioning (pun intended) away from react-native-modal.

barring digging into react-native-modal innards and filing issues against that library...anything I / we can do on the RNW side @imnotjames? the odd thing is that react-native-modal (obviously given its high profile) works on react-native proper, so some quirk of the native Modal implementation appears to be resilient to the issues you've flagged.

It's something to do with react-native-web and the library https://github.com/oblador/react-native-animatable - it MIGHT have to do with the modal implementation but as far as I can see that's not the case with how it's operating.

If someone digs further and finds it related to the modal implementation after all I'd be happy to assist fixing it. Just seems somewhat unrelated, unfortunately.

@viggyfresh I dug into it some - is it possible that with react-native the items within a modal are ALWAYS mounted if the Modal is mounted?

Per https://github.com/react-native-community/react-native-modal/blob/master/src/modal.tsx#L766 it kinda seems like it expects open() to be called & a ref to items within the contents of the modal to already be set - as if the contents of the modal were mounted if the modal were ever mounted.

With the implementation I'd created - when the items within the modal are not visible they are all entirely unmounted.

With the implementation I'd created - when the items within the modal are not visible they are all entirely unmounted.

when change visible from true to false, the Modal in react-native will not unmounted, it still there

@viggyfresh I dug into it some - is it possible that with react-native the items within a modal are ALWAYS mounted if the Modal is mounted?

Per https://github.com/react-native-community/react-native-modal/blob/master/src/modal.tsx#L766 it kinda seems like it expects open() to be called & a ref to items within the contents of the modal to already be set - as if the contents of the modal were mounted if the modal were ever mounted.

With the implementation I'd created - when the items within the modal are not visible they are all entirely unmounted.

hey @imnotjames - i think your supposition is correct. prior art for react-native-web modals (see https://github.com/Dekoruma/react-native-web-modal/tree/master/packages) keep child content mounted even when the modal is not visible - we figured this out when we had issues with selenium tests finding the "wrong" button to click. so I think keeping children mounted is actually correct?

Ok - #1646 has been updated to always have the children mounted - but hidden when not visible.

I'm still confused with why the react-native-modal extension always expects children to be mounted. The react-native Modal implementation does not seem to mount children when visible & when made invisible again will unmount them.

This is per https://github.com/facebook/react-native/blob/master/Libraries/Modal/Modal.js#L187 - which is why I was confused in this being a problem.

Perhaps the implementation here doesn't match up in some other way? Or maybe I'm misinterpreting the code?

I'm still confused with why the react-native-modal extension always expects children to be mounted. The react-native Modal implementation does not seem to mount children when visible & when made invisible again will unmount them.

This is per https://github.com/facebook/react-native/blob/master/Libraries/Modal/Modal.js#L187 - which is why I was confused in this being a problem.

Perhaps the implementation here doesn't match up in some other way? Or maybe I'm misinterpreting the code?

yeah this is weird, because given this, I'm a little confused as to how react-native-modal even works? is it possible its an unmount / rerender timing thing that works out in react-native-modal's favor on native but doesn't work on web?

Confirmed via https://snack.expo.io/ that no children are mounted in RN when the Modal is not visible. And onDismiss timing is aligned with that too.

Should I revert the change that fixes integration react-native-modal to try to be more in-line with the react-native implementation?

I think it would be worth understanding why react-native-modal behaves differently with the web implementation

Per previous investigation into this - the big issue is that open() on react-native-modal is being called when the react-native-animatable ref hasn't been properly mounted.

Looking at previous implementation.. it COULD be that we're using useEffect to maintain a single state flag for whether the children should be mounted or not. There's still some weirdness that I don't entirely understand..

| Step | React-Native | React-Native-Web |
| --- | --- | --- |
| 0 | Set Visible Prop to True | Set Visible Prop to True |
| 1 | Modal instantly flips to mount children but they are not visible - contentRef gets set | useEffect hook is called & sets isRendering to true for next render |
| 2 | open() gets called (animates) | open() gets called (and leaves isTransitioning flag set, doesn't animate because not mounted) |
| 3 | IDLE (native code handles animations) | reRender of Modal mounts children with web animations - contentRef gets set |
| 4 | IDLE | IDLE |
| 5 | Set Visible Prop to False | Set Visible Prop to False |
| 6 | Modal instantly flips to unmount children | useEffect hook is called but nothing happens - animations start firing |
| 7 | ??? Somehow the native code keep the children mounted while animating? :thinking: | animation completes and sets isRendering to false |
| 8 | children actually unmount | children unmount |

We can't do exactly what react-native does because of step 7, effectively. If we did we'd end up with no animations when dismissing the modal.

Perhaps adding an extra bit to the modal would give us the best of both worlds? Something like..

if (!visible && !isRendering) {
  return null;
}

That way we will have the children mounted for as long as the isRendering flag is true or the visible prop is true. This should allow us to get the animatable pieces for react-native-modal mounted early enough for their open() function to operate on it.

I'll need to set up a minimal implementation to get react-native-modal set up to run with that kind of a patch to check if this corrects the issue on the browser.

Thanks. Yeah there are definitely bigger problems with lack of exit animations for React web. We should just do what we can here, with the bias towards an optimized web experience.

:partying_face: The PR for Modal is now merged into @next - so the canary should have that good to go!

Seems to be all set with compatibility with react-native-modal (see here) & next.js and SSR.

@imnotjames I've found a bug, in your example, if you change isVisible to be always true, backdrop won't show. Do you know if this is related to react-native-modal or to the web implem?

EDIT: tried on native, and it does work OK fyi

I see that too - I had to change it and then refresh because of some neat hot-reloading :) But I do see that.

Confirmed it's the same between 0.0.0-80dae21e2 and canary (12 days ago vs now)

Looks like the background IS there but it maintains an opacity of 0 - which isn't visible. This is likely becuase the animation doesn't fire off. The other animations also do not fire off.

This is because open in react-native-modal fires off when componentDidMount() but at that time none of the refs have been set yet for some reason.

I'm not sure if that's caused by the modal implementation or caused by something else, though..

Looks like ref gets called twice - ref is called with null, then mount is called then, the actual node sent to the ref.


When experimenting with a smaller test case the refs do get set before the mount so it could be something up with the modal..

  test('ref is set before `mount` hook', () => {
    const spy = jest.fn();

    function TestComponent() {
      React.useEffect(() => spy("mount"), []);
      return <View ref={(ref) => ref ? spy("ref") : spy("noref")} />;
    }

    render(<TestComponent />)

    expect(spy).toHaveBeenNthCalledWith(1, "ref");
    expect(spy).toHaveBeenNthCalledWith(2, "mount");
  });

However, when using a modal inside the component it's the other way around - mount then ref - so something must be up with how that's getting set.

@Titozzz how did you confirm this on native? Would you be able to check if componentDidMount() does have the ref for backdropRef and contentRef already set?

I've just used react-native-modal on iOS, nothing special (with isVisible set to true) and it was animating on mount so the background was set

Ok - I think I got it - I think the cause is that in ModalPortal we useEffect to create the portal element & don't render the View until that happens.

This means that everything mounts first, the parent (the enhanced modal) fires its mount function, tries to animate but can't, and then everything just pops in.

One possible way to fix this is a.. little ugly. We create the portal's element outside of useEffect conditionally and refactor ModalPortal a bit.

@necolas I have implemented this at https://github.com/necolas/react-native-web/pull/1750 - open to any suggestions on improvements & not sure the process for merging this into the next branch (probably amending it to the original modal commit once it's ready?)

@Titozzz is there any way you could test with the changes at https://github.com/necolas/react-native-web/pull/1750 to see if they help? I'm having trouble getting it into code sandbox or anything like that

PS @Titozzz you rock for finding this :) Thank you.

Thanks for investigating I'll test the changes.

@imnotjames I can confirm it works in that use case with your fix!

Having trouble with an input inside a modal - the trap doesn't allow it to focus for me, it shows document.activeElement to be the last active element, not the one that's receiving focus.

Edit: It's not the trap actually, perhaps this is a React event system bug then. It seems to not "get" to the input. Using react/dom 0.0.0-experimental-94c0244ba.

Oh no! That doesn't sound great.

If you think it could be related to the modal code, could you create a replication of this in codesandbox.io? I'd be happy to dig into it sometime if it looks like that's the problem.

If not, oof, that's no fun..

Latest canary includes the patch @imnotjames wrote to fix the issue @Titozzz reported. Thanks

Is there any more feedback on this build? Especially the change to the default flex-basis value.
If you're on 0.13, this update has no API changes and should be easy to trial.

cc @EvanBacon @comp615

Is there any more feedback on this build? Especially the change to the default flex-basis value.

If you're on 0.13, this update has no API changes and should be easy to trial.

cc @EvanBacon @comp615

we've been using it in prod for some time - no complaints!

the flex-basis default change hasn't bitten us in any instances where we share code between React Native and React Native Web, but did cause a few visual regressions in a React Native Web-only project. as documented in the changelog, a few style modifications fixed this.

Thanks @viggyfresh

a few visual regressions in a React Native Web-only project...a few style modifications fixed this.

Please can you go into detail? It might help me provide recommendations in the release notes.

Here's a temp link to a version of Twitter with the latest canary (7cbe1609b): https://mobile.twitter.com/?tts_token=HgXOZZ7g4QHfBdPw, I used it for a few minutes and didn't encounter any major issues. Hopefully that's a positive indication for now. We're still trying to get 0.13 so a little behind, but hopefully we can get some QA and canaries on the new versions soon!

Thanks! I need to update the canary to pick up the patch in 0.13.17 that we worked through too

Thanks @viggyfresh

a few visual regressions in a React Native Web-only project...a few style modifications fixed this.

Please can you go into detail? It might help me provide recommendations in the release notes.

I did have the same problem on a date picker where days would take more width than on native. flexBasis: 0, to styles fixed that.

Thanks @viggyfresh

a few visual regressions in a React Native Web-only project...a few style modifications fixed this.

Please can you go into detail? It might help me provide recommendations in the release notes.

I did have the same problem on a date picker where days would take more width than on native. flexBasis: 0, to styles fixed that.

@necolas sorry I missed your response - but my situation was similar to @RichardLindhout. At the risk of oversimplifying a fairly complex view hierarchy, in situations where we had 3 evenly-flexed columns, if dynamically loaded content overflowed the initial layout boundaries, the flexed columns were no longer even (when they previously were). going back to flexBasis: 0 got us back to the original (pre-canary) behavior.

Thanks That's sounds like what I noticed when first developing RNW and including the calculator example. To get columns of equal width needs either flexBasis:0% or a derived percentage (e.g. 4 cols needs 25%)

Is there a way to implement Pressable's hover using hooks, without creating a new DOM node?

For example, can instead of this:

const HoverText = () => (
  <Pressable>{({ hovered }) => <Text style={{ color: hovered ? 'blue' : 'green' }}>Hi</Text>}</Pressable>
)

Could I do something like this?

const [hovered, bindings] = useHovered()

return (
  <Text {...bindings} style={{ color: hovered ? 'blue' : 'green' }}>
    Hi
  </Text>
)

I'm looking to create generalizable components that can be styled based on their hover state. However, having them wrapped with Pressable means an extra DOM node and more styles to account for, which can get confusing when reusing the component throughout the app.

Thanks!

Was this page helpful?
0 / 5 - 0 ratings