Skip to content

fix: Return closest Text matching a query #489

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

Merged
merged 7 commits into from
Aug 16, 2020

Conversation

AugustinLF
Copy link
Collaborator

Summary

When using getByText() on a component using nested Texts, it should match the closest one.

<Text id={1}>
  <Text id={2}>My text</Text>
</Text>

I've found this bug when trying to upgrade from @5.X to @7.X, this breaks one of my tests where an onPress handler is attached to the lower Text, and since getByText returns its parent, using fireEvent.press(getByText('My text')) doesn't work.

Test plan

The added tests should show the covered case. I added an another test to be sure I would not introduce a regression.

@AugustinLF AugustinLF force-pushed the fix/get-closes-text-when-nested branch from b5be945 to 0bdab5d Compare August 7, 2020 11:39
@AugustinLF
Copy link
Collaborator Author

@thymikee Is there anything I could do to help this PR to move forward? It prevents us from upgrading to the latest version :/

Copy link
Member

@thymikee thymikee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me!

Copy link
Member

@mdjastrzebski mdjastrzebski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice edge case handling.

I've added two minor suggestions to improve the code comments & test.

@thymikee thymikee merged commit 9f6ca56 into callstack:master Aug 16, 2020
@thymikee
Copy link
Member

Thank you @AugustinLF !

@AugustinLF AugustinLF deleted the fix/get-closes-text-when-nested branch August 17, 2020 12:46
@AugustinLF
Copy link
Collaborator Author

Thanks a lot, was a pleasure to help get that in, thanks for the final fixes!

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