prscX / react-native-tooltips

React Native: Native Tooltip View
Apache License 2.0
271 stars 40 forks source link

Incorrect code? #6

Closed aMarCruz closed 5 years ago

aMarCruz commented 6 years ago

@prscX , thanks for your great work.

Looking at the JS code I see this block stating at line 72 of RNTooltips.js:

    if (props.text === undefined) {
      props.text = Tooltips.defaultProps.text;
    } else if (props.position === undefined) {
      props.position = Tooltips.defaultProps.position;
    } else if (props.align === undefined) {
       // . . . etc

If the intention is to test each value, the correct code would be:

    if (props.text === undefined) {
      props.text = Tooltips.defaultProps.text;
    }
    if (props.position === undefined) {
      props.position = Tooltips.defaultProps.position;
    }
    if (props.align === undefined) {
       // . . . etc

...otherwise the chain will break with the first equality, ex. if both position and align are undefined, only position will get the default.

In the other hand, React and React native already sets automatically default values with the static Tooltips.defaultProps property, so these block is not necessary (EDIT: sorry, Show is static as well).

prscX commented 6 years ago

Thanks a lot @aMarCruz for pointing out the issue.

Completely my bad. Please let me know in case you are looking to raise a PR else I can do the required changes in order fix it.

Thanks </ Pranav >

aMarCruz commented 6 years ago

@prscX , just now I'm working in a local fork of your repo, fixing a strange issue with the Android API. I need to finish my current project this week, but after that I will gladly contribute fixing this new issue. Thanks

prscX commented 5 years ago

Hi @aMarCruz: While fixing ISSUE: 9, I have fixed this as well.

Thanks for sharing the issue.

Thanks </ Pranav >