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

VPN-4684: Add privacy features help sheet #8878

Merged
merged 7 commits into from
Jan 19, 2024
Merged

VPN-4684: Add privacy features help sheet #8878

merged 7 commits into from
Jan 19, 2024

Conversation

MattLichtenstein
Copy link
Contributor

Description

NOTE: Depends on #8864: VPN-4687: Add DNS settings help sheet

  • Implement help sheet for privacy features
Screen.Recording.2023-12-28.at.3.20.42.PM.mov

Reference

VPN-4684: Implement the 'Privacy features' in-product help sheet

value: Privacy features
comment: Title label for the privacy features help sheet
privacyHeader:
value: What do privacy features do for me?
Copy link
Collaborator

Choose a reason for hiding this comment

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

"What do…do" doesn't sound great?

"What can…do" looks like a minimal change here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

Yes, I think this is a good suggestion, especially because "do" implies they do it by default, but "can" is more aligned with the fact that users can turn things off or on here.

Anyway, I agree, I think we should update to "What can privacy features do for me?"

src/translations/strings.yaml Outdated Show resolved Hide resolved
Copy link

Uh oh! Looks like an error! Details

Client ID static/taskcluster/github does not have sufficient scopes and is missing the following scopes:

{
  "AnyOf": [
    "queue:rerun-task:mozillavpn-level-1/fluFxJRJSkyHHzyo2eYFoQ/HNvzCTmnQwWmiWEeyAnmKA",
    "queue:rerun-task-in-project:none",
    {
      "AllOf": [
        "queue:rerun-task",
        "assume:scheduler-id:mozillavpn-level-1/fluFxJRJSkyHHzyo2eYFoQ"
      ]
    }
  ]
}

This request requires the client to satisfy the following scope expression:

{
  "AnyOf": [
    "queue:rerun-task:mozillavpn-level-1/fluFxJRJSkyHHzyo2eYFoQ/HNvzCTmnQwWmiWEeyAnmKA",
    "queue:rerun-task-in-project:none",
    {
      "AllOf": [
        "queue:rerun-task",
        "assume:scheduler-id:mozillavpn-level-1/fluFxJRJSkyHHzyo2eYFoQ"
      ]
    }
  ]
}

  • method: rerunTask
  • errorCode: InsufficientScopes
  • statusCode: 403
  • time: 2023-12-29T17:30:58.740Z

onClicked: helpSheetLoader.active = true

accessibleName: MZI18n.GlobalHelp
Accessible.ignored: !visible
Copy link
Contributor

Choose a reason for hiding this comment

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

MZButtonBase has Accessible.ignored: !visible this already. Same comment for related PRs.

Comment on lines +937 to +980
privacyBody1:
value: Privacy features give you greater control over the types of content you see, helping you stay safe online.
comment: Body label for the privacy features help sheet
privacyBody2:
value: If you activate these features, they’ll overwrite any custom DNS you’re using. These protections are not a substitute for taking other security precautions — for example, while blocking malware you should still avoid downloading attachments in strange emails.
comment: Body label for the privacy features help sheet
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Do we need to break up the body into two strings? Is it more flexible for localization if a single string (and a single Text field in the UI) is used?

Feel free to ignore this comment.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In this case, one string provides more flexibility for translation.

Copy link
Contributor Author

@MattLichtenstein MattLichtenstein Jan 2, 2024

Choose a reason for hiding this comment

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

To get the UI to match the mocks layout/spacing, we have to use two text components here

Copy link
Member

@lesleyjanenorton lesleyjanenorton left a comment

Choose a reason for hiding this comment

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

Thanks Matt! Same URL comment here: We just need to remove the locale from the URL so that moz.org can route to the correct page based on the user's browser language setting.

@@ -1381,6 +1381,11 @@ void MozillaVPN::registerUrlOpenerLabels() {
uo->registerUrlLabel("upgradeToAnnualUrl", []() -> QString {
return Constants::upgradeToAnnualUrl();
});

uo->registerUrlLabel("sumoPrivacy", []() -> QString {
return "https://support.mozilla.org/en-US/kb/"
Copy link
Member

Choose a reason for hiding this comment

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

Same comment here, let's remove "en-US" so that moz.org can determine and reroute to the correct locale.

{type: MZHelpSheet.BlockType.LinkButton, text: MZI18n.GlobalLearnMore, margin: 16, action: () => { MZUrlOpener.openUrlLabel("sumoPrivacy") } },
]

onClosed: helpSheetLoader.active = false
Copy link
Member

Choose a reason for hiding this comment

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

Do we anticipate these help sheets existing outside of Loaders? Any reason to not create a separate component for the Loader/HelpSheet duo? Seems like we could save ourselves a bit of duping.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agree. see here

@MattLichtenstein MattLichtenstein merged commit 5a4d87a into main Jan 19, 2024
125 checks passed
@MattLichtenstein MattLichtenstein deleted the VPN-4684 branch January 19, 2024 13:20
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.

5 participants