Skip to content

Update Alert API Examples in RNTester#29855

Closed
anku255 wants to merge 3 commits into
facebook:masterfrom
MLH-Fellowship:mlh/new-alert-screen
Closed

Update Alert API Examples in RNTester#29855
anku255 wants to merge 3 commits into
facebook:masterfrom
MLH-Fellowship:mlh/new-alert-screen

Conversation

@anku255
Copy link
Copy Markdown
Contributor

@anku255 anku255 commented Sep 3, 2020

Summary

This PR updates the Alert API Examples in RNTester. It is part of the MLH Fellowship.

Changelog

  • Added new use case for the Alert API.
  • Wrote each use case in a separate example block to allow searching
  • Wrote Detox tests for the Alert Screen

Test Plan

Screen Recording: https://i.imgur.com/9ubMmDC.mp4

  • Run the RNTester app locally by following these instructions.
  • Search for Alerts and tap on it

Screenshot 2020-07-07 at 1 29 03 PM

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 3, 2020
@anku255
Copy link
Copy Markdown
Contributor Author

anku255 commented Sep 3, 2020

@rickhanlonii I have created the PR for the Alert Screen. Please take a look :)

@pull-bot
Copy link
Copy Markdown

pull-bot commented Sep 3, 2020

Messages
📖

📋 Verify Changelog Format - A changelog entry has the following format: [CATEGORY] [TYPE] - Message.

DetailsCATEGORY may be:
  • General
  • iOS
  • Android
  • JavaScript
  • Internal (for changes that do not need to be called out in the release notes)

TYPE may be:

  • Added, for new features.
  • Changed, for changes in existing functionality.
  • Deprecated, for soon-to-be removed features.
  • Removed, for now removed features.
  • Fixed, for any bug fixes.
  • Security, in case of vulnerabilities.

MESSAGE may answer "what and why" on a feature level. Use this to briefly tell React Native users about notable changes.

Generated by 🚫 dangerJS against d12316a

@analysis-bot
Copy link
Copy Markdown

analysis-bot commented Sep 3, 2020

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,210,209 748
android hermes armeabi-v7a 6,859,385 752
android hermes x86 7,644,841 744
android hermes x86_64 7,535,785 744
android jsc arm64-v8a 9,369,602 512
android jsc armeabi-v7a 9,010,910 516
android jsc x86 9,232,322 508
android jsc x86_64 9,809,470 516

Base commit: d6fef50

@analysis-bot
Copy link
Copy Markdown

analysis-bot commented Sep 3, 2020

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: d6fef50

Copy link
Copy Markdown
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.

Looks good, can you include a screenshot or demo in the test plan?

* @format
*/

/* global device, element, by, expect, waitFor */
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This shouldn't be necessary because they're configured in the eslint/flow settings. Is that not working?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, it is not working.

Screenshot 2020-09-10 at 9 58 07 PM

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

element may not be configured but device and expect should be?

await element(by.label('Back')).tap();
});

it('should show alert dialog with message and default button', async () => {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ideally these test descriptions would specify which example they're testing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I am not sure how I should add them. I have pushed one commit. Please let me know if that's the correct way to do it.

Comment thread packages/rn-tester/js/examples/Alert/AlertExample.js
@anku255
Copy link
Copy Markdown
Contributor Author

anku255 commented Sep 10, 2020

@rickhanlonii Thanks for the review! I have added the screen-recording in the PR description. Please let me know if you need anything else.

Copy link
Copy Markdown
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
Copy Markdown
Collaborator

This pull request was successfully merged by @anku255 in 1b943a9.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Sep 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants