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

Only use native driver if on iOS #10

Closed
wants to merge 1 commit into from

Conversation

wli
Copy link

@wli wli commented Nov 29, 2017

Only use native driver if on iOS

This is due to an underlying react-native bug where strings aren't evaluated properly when using native drivers.
facebook/react-native#14161

This is due to an underlying react-native bug where strings aren't
evaluated properly when using native drivers.
facebook/react-native#14161
@isnifer
Copy link

isnifer commented Dec 27, 2017

@doomsower It would be nice to merge this PR :)

@ShMcK
Copy link

ShMcK commented Jan 5, 2018

@doomsower, anything holding back this PR ?

For others with this issue, for now you can npm i @wli/react-native-modal-popover.

@wli
Copy link
Author

wli commented Jan 5, 2018

Thanks @ShMcK, I mainly published that for my own use. I did manage to get it working in my app with my fork, but keep in mind that it's for short-term use only (until this PR is merged). It won't really be kept up to date with merges to this repo.

On an mostly-unrelated note (don't know where to properly report this), if you found this PR and are using Expo and/or react-native, you might also have run into the issue of the popover not vertically aligning correctly. I'm not sure which library is as fault (and I don't have time to really dive into it right now), but I locally overrode the Popover with this custom class. Hopefully this will help someone:

// A local file called Popover.js
import Expo from 'expo';
import React from 'react';
import {
  Platform,
  StyleSheet,
} from 'react-native';
import BasePopover from '@wli/react-native-modal-popover';


// Android is not respecting the statusBarHeight properly in calculations.
// Therefore, it's rendering ~24 pixels lower than intended, and this is a dirty
// hack to get it working again.
class Popover extends React.PureComponent {
  // We need to name this Popover because PopoverTouchable expects a component
  // with the name Popover.
  static displayName = 'Popover';

  render() {
    return (
      <BasePopover
        {...this.props}
        arrowStyle={[styles.arrow, this.props.arrowStyle]}
        contentStyle={[styles.content, this.props.contentStyle]}
      />
    );
  }
}


const styles = StyleSheet.create({
  content: {
    marginTop: Platform.OS === 'ios' ? 0 : -Expo.Constants.statusBarHeight,
    borderRadius: 8,
  },
  arrow: {
    marginTop: Platform.OS === 'ios' ? 0 : -Expo.Constants.statusBarHeight,
  },
});


export default Popover;

and then just import the modified Popover when you need to use it.

import {PopoverTouchable} from '@wli/react-native-modal-popover';
import Popover from '../components/Popover';

@doomsower
Copy link
Owner

I am back, I published 0.0.4 version of this lib, and with it I brought back my old workaround for this bug. So it should now useNativeDriver both on iOS and Android. Example project worked for me on both platforms. So I'm closing this for now.
If it doesn't - please create issue.
Thank you.

@doomsower doomsower closed this Jan 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants