Skip to content
This repository has been archived by the owner on Jan 14, 2020. It is now read-only.

[DEV-4168] Fix Links #349

Merged
merged 1 commit into from
Oct 28, 2019
Merged

[DEV-4168] Fix Links #349

merged 1 commit into from
Oct 28, 2019

Conversation

richarddubay
Copy link
Member

DESCRIPTION

What does this PR do, or why is it needed?

This PR fixes some of the openUrl links so that they have the correct arguments. This includes a fix to the link for the live banner.

How do I test this PR?

Open the sim while things are live. Make sure that the live banner shows up, and that when you tap on it that Church Online opens up in an in-app browser.

TODO

  • I am affirming this is my best work (Ecclesiastes 9:10)
  • PR has a relevant title that will be understandable in a public changelog (ie...non developers)
  • No new warnings in tests, in storybook, and in-app
  • Upload GIF(s) of iOS and Android if applicable
  • Set a relevant reviewer

REVIEW

  • Review updates to test coverage and snapshots
  • Review code through the lens of being concise, simple, and well-documented

Manual QA

  • Manual QA on iOS and ensure it looks/behaves as expected
  • Manual QA on Android and ensure it looks/behaves as expected

The purpose of PR Review is to improve the quality of the software.

@richarddubay richarddubay changed the title Fix Links [DEV-4168] Fix Links Oct 27, 2019
@@ -88,7 +88,11 @@ class UserSettings extends PureComponent {
<TableView>
<Touchable
onPress={() =>
openUrl('https://newspring.cc/privacy')
Copy link
Contributor

Choose a reason for hiding this comment

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

this means changing the WebBrowser did infact introduce breaking changes, that should be fixed in core too.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, we just never changed these links to include the necessary arguments. What do we need to fix in core? Default to empty strings if the argument isn't there or something?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, it should be able to handle blank second and third arguments

@redreceipt redreceipt merged commit 7929094 into develop Oct 28, 2019
@redreceipt redreceipt deleted the fix-live branch October 28, 2019 00:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants