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

Incorrect code? #6

Closed
aMarCruz opened this issue Jun 28, 2018 · 3 comments
Closed

Incorrect code? #6

aMarCruz opened this issue Jun 28, 2018 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@aMarCruz
Copy link

aMarCruz commented Jun 28, 2018

@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 prscX self-assigned this Jun 29, 2018
@prscX
Copy link
Owner

prscX commented Jun 29, 2018

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 >

@prscX prscX added the bug Something isn't working label Jun 29, 2018
@aMarCruz
Copy link
Author

@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
Copy link
Owner

prscX commented Sep 8, 2018

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

Thanks for sharing the issue.

Thanks
</ Pranav >

@prscX prscX closed this as completed Sep 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants