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.

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

Web Support #82

Open
SteffeyDev opened this issue Sep 5, 2020 · 27 comments
Open

Web Support #82

SteffeyDev opened this issue Sep 5, 2020 · 27 comments

Comments

@SteffeyDev
Copy link
Owner

Right now, this library may or may not work on web depending on how it is used. I never intended to officially support web, and have not setup any testing for web. However, if someone would like to submit a PR for web support and testing, I would be willing to work with them to incorporate it going forwards.

@nandorojo
Copy link

I'm interested in using this for web. React Native Web added support for modals in 0.14, is that all that's needed? https://github.com/necolas/react-native-web/releases/tag/0.14.0

@SteffeyDev
Copy link
Owner Author

Ah, that's actually an important development. It sounds like the other issue with web is PropTypes, check out #86, maybe you can contribute to the discussion there.

@nandorojo
Copy link

I'll be trying this out on web now that I've upgraded to RNW 0.14.

@nandorojo
Copy link

nandorojo commented Oct 23, 2020

I solved #86 locally with the solution I posted.

Web measurement issue

As for normal web support: there is a weird bug, where the popover content is measured entirely incorrectly. I'm using RNW 0.14 with modal support. It seems like the measurement of the host node is off. It's at the top of my screen, but for some reason it's measuring it like 2000px down the screen.

Video

I took a video to demonstrate what's happening. On native, no issues. On web, as you'll see in the logs (in addition to some of my own logs), the y value is 2000+ px, which seems to send the div all the way down. I zoom out to show what's happening on the screen.

Snippet

I'm trying to make a tooltip, so I made this component:

import React from 'react'
import { View, Pressable } from 'react-native'
import Popover from 'react-native-popover-view/src/Popover' // for debugging

export default function Tooltip(props: Props) {
  const { children, content, ...press } = props

  return (
    <Popover
      debug
      from={(ref, show) => (
        <Pressable {...press} onPress={show}>
          <View ref={ref}>{children}</View>
        </Pressable>
      )}
    >
      <View>{content}</View>
    </Popover>
  )
}

And I'm using it like so in my component:

export default () => (
  <Tooltip content={<Text style={{ color: 'white', height: 500, width: 500, backgroundColor: 'blue' }}>Hey!</Text>}>
    <Text sx={{ color: 'yellow' }}>Hi</Text>
  </Tooltip>
)

SteffeyDev added a commit that referenced this issue Oct 24, 2020
@SteffeyDev
Copy link
Owner Author

Ok, so I got it kind of working. The popover renders in the correct position, but I'm having some issue with the styling. react-native-web does not seem to be preserving the styles that I am applying from my StyleSheet (via array or destructing into an object), which means that the background is not showing up at all. When I dig around in the DOM and apply the styles manually, it actually works OK, which is cool. Not sure if it is an issue with react-native-web itself or maybe just with the new Modal component. The specific styles that are missing are the position: absolute; and background-color, along with some related properties.

@SteffeyDev
Copy link
Owner Author

Hopefully you can investigate a bit and let me know if there's anything else I need to do on my end. I'm invested into this now and would be excited to get this working :)

@SteffeyDev
Copy link
Owner Author

I released the changes under 3.1.0, so those are live on NPM for you to test.

@cmaycumber
Copy link

Web support would be super awesome! I've started using this on my own application but am having trouble with a popover that's inside of a custom header rendered by react-navigation because the onRequestClose function doesn't fire.

Any ideas? Building a custom popover is something I've been toying with the last few days and this library definitely ticks all the boxes.

@nandorojo
Copy link

@SteffeyDev is the measurement breaking for you too, or just the color and position? (Or is the position what's incorrectly placing it?)

One possibility, I assume you aren't using onLayout to measure, but if you are, I believe this behaves a bit differently in web. If you're using measure or measureInWindow, I can try to investigate differences in behavior.

@nandorojo
Copy link

@cmaycumber that's interesting, I would assume it should still overlay the entire app since it's in a modal. I wonder what would happen if you tried the react-native-elements tooltip to see if you get the same issue.

@SteffeyDev
Copy link
Owner Author

@cmaycumber I have made this popover launch out of a custom react-navigation headers before, so I know it works and is possible. I was using my https://github.com/SteffeyDev/react-navigation-popover library, which was a wrapper around react-navigation and this Popover component, you can kind of see what I was doing here https://github.com/SteffeyDev/react-native-popover-view-test-app/blob/master/App.js. I don't maintain that repo anymore due to lack of interest and how fast react-navigation was changing things, but maybe it can help. If you have additional issues along this line not related to web support, please open another issue, so that this can stay focused on web.

@nandorojo The measurement and placement is working perfectly actually, its just the styles that are not getting applied correctly. Once I manually adjust the styles it seems to work fine (except for it not accounting for scroll behavior).

@nandorojo
Copy link

Got it. Maybe we could test toggling between measure and measureInWindow to see if that accounts for the scroll? I'll take a look at RNW's code for that.

@cmaycumber
Copy link

cmaycumber commented Nov 3, 2020 via email

@cmaycumber
Copy link

@SteffeyDev @nandorojo Thanks for the direction. Super helpful.

I'll definitely take a look at your example.

I am using it on the web so I'll just create a new issue with this one mentioned if the problem persists!

@nandorojo
Copy link

I’d ideally like the elements outside of the Popover to be still clickable but close the popover when they are clicked.

@cmaycumber oh got it, I misunderstood your intention. I'm not sure what the move is in the case of wanting to capture other touches below the modal.

If your other elements capture touches when you have JS_MODAL set, maybe you could store the popover visibility in a global context and then set the visible to false when one of the other items is pressed. I assume that's not ideal, though.

@cmaycumber
Copy link

If your other elements capture touches when you have JS_MODAL set, maybe you could store the popover visibility in a global context and then set the visible to false when one of the other items is pressed. I assume that's not ideal, though.

Yeah, exactly my thought. I was trying to avoid that but it might be impossible.

@SteffeyDev
Copy link
Owner Author

@nandorojo I should have been more specific. The popover does launch from the correct position regardless of scroll, it just doesn't move to stay pointing at the element when the page is scrolled after the popover is open. Not a huge deal. Bigger issue right now is styling.

@SteffeyDev
Copy link
Owner Author

@cmaycumber my code won't be much help for that use case, I didn't realize you wanted things in the background to be clickable either. No idea how to handle that elegantly. Web has the concept of focus but mobile does not.

@nandorojo
Copy link

Still trying to test this out on my end.

For context, I'm looking to recreate this tooltip/popover from React Geist. Code here.

@martinjuhasz
Copy link

I just gave it a quick try on web by dropping it into my project. This seems to not work on my end. Native is fine, but on the web the popover content is shown all the time, misplaced, and clicking the Touchable does change nothing at all.

Will see if i can debug this, to get a little bit more info what is failing here.

@suresh-legato
Copy link

@cmaycumber my code won't be much help for that use case, I didn't realize you wanted things in the background to be clickable either. No idea how to handle that elegantly. Web has the concept of focus but mobile does not.

I have fixed that onRequestClose not firing issues. @SteffeyDev I could raise a PR for the same.
Issue is in setting the visibility.

@SteffeyDev
Copy link
Owner Author

I'll take any PRs :)

@nandorojo
Copy link

Has anyone made more progress on web?

@nandorojo
Copy link

This portal project looks promising, and seems like it hopes to work on web: https://github.com/gorhom/react-native-portal

Maybe a portal could power this library's popovers.

@jonathanj
Copy link

jonathanj commented Sep 22, 2022

I ran into an issue on web (RNW 0.18.9) where react-native-popover-view indicated it was waiting for a resize/orientation event to complete, but there wasn't anything like this happening and so it never completed.

It turns out that RNW will call change event listeners the first time Dimensions.get is used.

The process is roughly:

  • react-native-popover-view adds a dimensions change listener
  • react-native-popover-view calls Dimensions.get('window') to read dimensions
  • The first time this happens, RNW calls all change listeners as though the dimensions were changed (but they have not)
  • react-native-popover-view thinks it should be expecting a dimension/orientation change to happen, and seems to rely on onLayout being called again
  • No new layout happens, so react-native-popover-view does not render the popover content

A solution that worked for me, which I acknowledge is a hack, was to call Dimensions.get('window') as a side-effect before registering the Dimensions change event handler here. Maybe I could simply do this in my own code before anything else? RNW's behaviour seems like a bug to me.

@jonathanj
Copy link

jonathanj commented Sep 22, 2022

Bigger issue right now is styling.

I managed to unravel the styling problems, at least the ones I was having, and I'm happy with the results, I ran into two issues.

1. StyleSheet.flatten

RNW 0.18.x reworked the StyleSheet internals quite substantially, in my case react-native-popover-view's use of StyleSheet.flatten was broken on RNW. The RNW 0.18 release mentions changes around StyleSheet and the docs recommend against flatten, and against dynamically reading styles at runtime from results of StyleSheet.create, which is being done to read shadow styles, etc.

When I logged out the result of StyleSheet.flatten(this.props.popoverStyle), it was just an empty object, even though the style I defined in my stylesheet was definitely not empty.

A solution that worked for me was avoiding using StyleSheet.create for my popoverStyle styles, instead just using a plain JS object. I think the correct change for react-native-popover-view is to stop using StyleSheet.flatten to "rehydrate" RN styles, and rather have explicit styles for the aspects it wants to apply.

2. Shadow style is not applied

After fixing the above, I could see my styles being applied to my popover but there was no shadow style. Some poking around in the devtools and it looks like the element with the shadow style is a giant transparent overlay, whereas this element is the popover container. Moving the shadow style to the latter solved the issue, and works on mobile and web.

I suspect the reason the code works on mobile now is the quirky way shadows work on RN, and that the current behaviour is a bug that happens to work on mobile but not web.

@corysimmons
Copy link

The measure animation bug on web doesn't seem to happen on small-width viewports. As the viewport gets wider, the popover changes the side it appears on (going from bottom to right), and eventually when the viewport is a huge width the popover goes to the left and the animation bug happens.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants