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

Added filtering to RNTester example screens #22777

Closed
wants to merge 10 commits into from
Closed

Added filtering to RNTester example screens #22777

wants to merge 10 commits into from

Conversation

CodingItWrong
Copy link
Contributor

This PR adds filtering functionality to individual example screens of RNTester. This is useful for Detox testing of RNTester, since Detox requires elements to be visible on the screen before they can be interacted with. Instead of needing to scroll an arbitrary amount, the test can enter the name of the example to be tested, just as is done on the main screen. This will lead to simpler and more reliable E2E tests for long example screens. This PR doesn't add any automated tests using the filter; those will be added in a separate PR.

This is implemented by extracting the existing filtering functionality out of RNTesterExampleList into a shared RNTesterExampleFilter component that can be used both within RNTesterExampleList (the main screen) and RNTesterExampleContainer (the example screen).

simulator screen shot - iphone 8 - 2018-12-24 at 08 22 46

simulator screen shot - iphone 8 - 2018-12-24 at 08 22 51

Changelog:

[General] [Added] - Added filtering to RNTester example screens

Test Plan:

Run RNTester and tap into example screens. Confirm examples still display, and that typing into the filter field filters the list of examples.

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Tests This PR adds or edits a test case. labels Dec 24, 2018
Copy link

@analysis-bot analysis-bot left a comment

Choose a reason for hiding this comment

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

Code analysis results:

  • eslint found some issues.

RNTester/js/RNTesterExampleList.js Show resolved Hide resolved
Copy link

@analysis-bot analysis-bot left a comment

Choose a reason for hiding this comment

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

Code analysis results:

  • eslint found some issues.

RNTester/js/RNTesterExampleList.js Show resolved Hide resolved
Copy link
Member

@rickhanlonii rickhanlonii left a comment

Choose a reason for hiding this comment

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

LGTM, just left a comment

RNTester/js/RNTesterExampleContainer.js Show resolved Hide resolved
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@rickhanlonii has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Co-Authored-By: CodingItWrong <joshjustice@me.com>
Copy link

@analysis-bot analysis-bot left a comment

Choose a reason for hiding this comment

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

Code analysis results:

  • eslint found some issues.

RNTester/js/RNTesterExampleContainer.js Show resolved Hide resolved
RNTester/js/RNTesterExampleContainer.js Show resolved Hide resolved
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@rickhanlonii has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

@CodingItWrong merged commit 386c2ec into facebook:master.

@facebook facebook locked as resolved and limited conversation to collaborators Dec 30, 2018
@react-native-bot react-native-bot added the Merged This PR has been merged. label Dec 30, 2018
grabbou pushed a commit that referenced this pull request Jan 3, 2019
Summary:
This PR adds filtering functionality to individual example screens of RNTester. This is useful for Detox testing of RNTester, since Detox requires elements to be visible on the screen before they can be interacted with. Instead of needing to scroll an arbitrary amount, the test can enter the name of the example to be tested, just as is done on the main screen. This will lead to simpler and more reliable E2E tests for long example screens. This PR doesn't add any automated tests using the filter; those will be added in a separate PR.

This is implemented by extracting the existing filtering functionality out of `RNTesterExampleList` into a shared `RNTesterExampleFilter` component that can be used both within `RNTesterExampleList` (the main screen) and `RNTesterExampleContainer` (the example screen).

![simulator screen shot - iphone 8 - 2018-12-24 at 08 22 46](https://user-images.githubusercontent.com/15832198/50401564-4273a300-0755-11e9-9120-9bf8fbb70261.png)

![simulator screen shot - iphone 8 - 2018-12-24 at 08 22 51](https://user-images.githubusercontent.com/15832198/50401566-44d5fd00-0755-11e9-9637-6e5ddce1c476.png)

Changelog:
----------

[General] [Added] - Added filtering to RNTester example screens
Pull Request resolved: #22777

Reviewed By: TheSavior

Differential Revision: D13561744

Pulled By: rickhanlonii

fbshipit-source-id: cb120626a8e2b8440f88b871557c0b92fbef5edc
@grabbou
Copy link
Contributor

grabbou commented Jan 3, 2019

Note: This PR has been merged with failing tests that are getting fixed in here: #22861

I understand that our CI may be often returning false-positive results. I think we should make sure that - when we merge a PR that has failing tests - this is expected.

@rickhanlonii
Copy link
Member

Ah, my bad here, sorry about that!

t-nanava pushed a commit to microsoft/react-native-macos that referenced this pull request Jun 17, 2019
Summary:
This PR adds filtering functionality to individual example screens of RNTester. This is useful for Detox testing of RNTester, since Detox requires elements to be visible on the screen before they can be interacted with. Instead of needing to scroll an arbitrary amount, the test can enter the name of the example to be tested, just as is done on the main screen. This will lead to simpler and more reliable E2E tests for long example screens. This PR doesn't add any automated tests using the filter; those will be added in a separate PR.

This is implemented by extracting the existing filtering functionality out of `RNTesterExampleList` into a shared `RNTesterExampleFilter` component that can be used both within `RNTesterExampleList` (the main screen) and `RNTesterExampleContainer` (the example screen).

![simulator screen shot - iphone 8 - 2018-12-24 at 08 22 46](https://user-images.githubusercontent.com/15832198/50401564-4273a300-0755-11e9-9120-9bf8fbb70261.png)

![simulator screen shot - iphone 8 - 2018-12-24 at 08 22 51](https://user-images.githubusercontent.com/15832198/50401566-44d5fd00-0755-11e9-9637-6e5ddce1c476.png)

Changelog:
----------

[General] [Added] - Added filtering to RNTester example screens
Pull Request resolved: facebook#22777

Reviewed By: TheSavior

Differential Revision: D13561744

Pulled By: rickhanlonii

fbshipit-source-id: cb120626a8e2b8440f88b871557c0b92fbef5edc
rozele pushed a commit to microsoft/react-native-windows that referenced this pull request Jul 18, 2019
Summary:
This PR adds filtering functionality to individual example screens of RNTester. This is useful for Detox testing of RNTester, since Detox requires elements to be visible on the screen before they can be interacted with. Instead of needing to scroll an arbitrary amount, the test can enter the name of the example to be tested, just as is done on the main screen. This will lead to simpler and more reliable E2E tests for long example screens. This PR doesn't add any automated tests using the filter; those will be added in a separate PR.

This is implemented by extracting the existing filtering functionality out of `RNTesterExampleList` into a shared `RNTesterExampleFilter` component that can be used both within `RNTesterExampleList` (the main screen) and `RNTesterExampleContainer` (the example screen).

![simulator screen shot - iphone 8 - 2018-12-24 at 08 22 46](https://user-images.githubusercontent.com/15832198/50401564-4273a300-0755-11e9-9120-9bf8fbb70261.png)

![simulator screen shot - iphone 8 - 2018-12-24 at 08 22 51](https://user-images.githubusercontent.com/15832198/50401566-44d5fd00-0755-11e9-9637-6e5ddce1c476.png)

Changelog:
----------

[General] [Added] - Added filtering to RNTester example screens
Pull Request resolved: facebook/react-native#22777

Reviewed By: TheSavior

Differential Revision: D13561744

Pulled By: rickhanlonii

fbshipit-source-id: cb120626a8e2b8440f88b871557c0b92fbef5edc
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Tests This PR adds or edits a test case.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants