Skip to content

Conversation

@wzieba
Copy link
Contributor

@wzieba wzieba commented May 8, 2023

Closes: #8939

Description

This PR ads behavior of opening correct links upon user clicks.

Testing instructions

Open Privacy Settings:

  • tap on the Advertising Options link: assert "Tacking & Opt Outs" page is shown
  • tap on the Privacy Policy assert "Privacy Policy" page is shown
  • tap on the Cookie Policy assert "Cookie Policy" page is shown

Images/gif

Mon.May.8.20.22.28.CEST.2023.mp4
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@peril-woocommerce
Copy link

peril-woocommerce bot commented May 8, 2023

Warnings
⚠️ PR is not assigned to a milestone.
Messages
📖

This PR contains changes to Tracks-related logic. Please ensure the following are completed:
PR Author

  • The PR must be assigned the Tracks label
    PR Reviewer
  • The tracks events must be validated in the Tracks system.
  • Verify the internal tracks spreadsheet has also been updated.

Generated by 🚫 dangerJS

@wzieba wzieba linked an issue May 8, 2023 that may be closed by this pull request
3 tasks
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented May 8, 2023

You can test the changes on this Pull Request by downloading an installable build, or scanning this QR code:

@codecov-commenter
Copy link

codecov-commenter commented May 8, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (0b9e232) 43.07% compared to head (3eace9c) 43.08%.

Additional details and impacted files
@@                      Coverage Diff                      @@
##             feature/new_privacy_screen    #8980   +/-   ##
=============================================================
  Coverage                         43.07%   43.08%           
  Complexity                         3950     3950           
=============================================================
  Files                               825      824    -1     
  Lines                             43420    43410   -10     
  Branches                           5667     5665    -2     
=============================================================
  Hits                              18704    18704           
+ Misses                            23067    23057   -10     
  Partials                           1649     1649           
Impacted Files Coverage Δ
...src/main/kotlin/com/woocommerce/android/AppUrls.kt 0.00% <ø> (ø)
...merce/android/ui/prefs/PrivacySettingsViewModel.kt 0.00% <0.00%> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@wzieba wzieba marked this pull request as ready for review May 10, 2023 10:42
@wzieba wzieba requested review from a team and atorresveiga and removed request for a team May 10, 2023 10:42
@wzieba wzieba added the feature: privacy Related to the privacy choice projects label May 10, 2023
@wzieba wzieba mentioned this pull request May 10, 2023
1 task
Base automatically changed from issue/8938_more_privacy_options_row to feature/new_privacy_screen May 11, 2023 07:34
Copy link
Contributor

@atorresveiga atorresveiga left a comment

Choose a reason for hiding this comment

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

LGTM! :shipit:

onCookiePolicyClicked: () -> Unit,
modifier: Modifier = Modifier
) {
val privacyText =
Copy link
Contributor

@atorresveiga atorresveiga May 11, 2023

Choose a reason for hiding this comment

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

np: more than half of this composable is about creating the string with the links, maybe we can move this logic into a separate function. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Good idea, but because of the recent design change and lack of this text - I removed this method.

Comment on lines +32 to +49
PrivacySettingsPolicesScreen(
onPrivacyPolicyClicked = {
AnalyticsTracker.track(AnalyticsEvent.PRIVACY_SETTINGS_PRIVACY_POLICY_LINK_TAPPED)
ChromeCustomTabUtils.launchUrl(
requireActivity(),
AppUrls.AUTOMATTIC_PRIVACY_POLICY
)
},
onCookiePolicyClicked = {
AnalyticsTracker.track(
AnalyticsEvent.PRIVACY_SETTINGS_THIRD_PARTY_TRACKING_INFO_LINK_TAPPED
)
ChromeCustomTabUtils.launchUrl(
requireActivity(),
AppUrls.AUTOMATTIC_COOKIE_POLICY
)
}
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've decided not to introduce VM to such a simple view.

@wzieba wzieba merged commit 46061a7 into feature/new_privacy_screen May 11, 2023
@wzieba wzieba deleted the issue/8939_support_link_actions branch May 11, 2023 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: privacy Related to the privacy choice projects

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Privacy Choices] Link support docs in Privacy Screen

5 participants