VPN-4687: Add DNS settings help sheet #8864

MattLichtenstein · 2023-12-22T20:44:40Z

Description

Implement help sheet for DNS settings
Slight tweaks to help sheet UI

Screen.Recording.2023-12-22.at.3.43.15.PM.mov

Reference

VPN-4687: Implement the 'Custom DNS Settings' in-product help sheet

flodolo

Not blocking on English, but I think the title should be updated.

src/translations/strings.yaml

vinocher · 2024-01-02T18:00:28Z

src/translations/strings.yaml

+    value: What is custom DNS?
+    comment: Header label for the custom dns help sheet
+  dnsBody1:
+    value: Whenever you connect to a website, a DNS (domain name system) first turns the domain name (e.g. www.mozilla.org) into an IP address that allows your internet traffic to reach that destination.


Just checking if this content has been approved by UX? The help text doesn't seem to explain what 'resolve websites' means in the screen UI.

@traceyvantyghem

+1 @vinocher . We don't seem to be answering the question from dnsHeader 'What is a custom DNS?').

@MattLichtenstein I agree that updating the title makes sense (per Flod's suggestion).

I didn't work on this content (and don't know much about the subject matter), so I checked in with Chris, but he didn't have any additional updates. Chris is returning to content lead on VPN, so I guess we can leave the body content as-is.

vinocher · 2024-01-02T18:02:17Z

src/ui/screens/settings/ViewDNSSettings.qml

+                onClicked: helpSheetLoader.active = true
+
+                accessibleName: MZI18n.GlobalHelp
+                Accessible.ignored: !visible


MZButtonBase has Accessible.ignored: !visible, so this should not be needed.

vinocher · 2024-01-02T18:04:21Z

nebula/ui/components/MZMenu.qml

@@ -103,10 +103,11 @@ Item {

        sourceComponent: menuBar.rightButtonComponent
        onItemChanged: {
-            if (item instanceof Text) {
+            //item.item is for right button components that might be inside a loder itself


vinocher · 2024-01-02T18:14:42Z

nebula/ui/components/MZMenu.qml

@@ -103,10 +103,11 @@ Item {

        sourceComponent: menuBar.rightButtonComponent
        onItemChanged: {
-            if (item instanceof Text) {
+            //item.item is for right button components that might be inside a loder itself
+            if (item instanceof Text || item.item instanceof Text) {


For the item.item condition, should the first item be checked if it is an instance of Loader? Also, are any null checks required?

Hmm, null checks for what? item? If item is null, item instanceof Text will evaluate to false

Please ignore the null check in the comment.

vinocher · 2024-01-02T18:23:49Z

nebula/ui/components/MZHelpSheet.qml

@@ -82,7 +81,8 @@ MZBottomSheet {
                        id: icon
                        anchors.centerIn: parent

-                        sourceSize.width: MZTheme.theme.iconSize
+                        source: "qrc:/nebula/resources/tip-filled.svg"


Was the iconSource property alias removed because MZHelpSheet is expected to always use tip-filled.svg?

yep, exactly.

vinocher · 2024-01-02T18:42:13Z

nebula/ui/components/MZHelpSheet.qml

                    Layout.rightMargin: MZTheme.theme.windowMargin / 2

                    Layout.preferredHeight: MZTheme.theme.rowHeight
                    Layout.preferredWidth: MZTheme.theme.rowHeight

+                    //Hacky workaround because for some reason, when opening a sheet via
+                    //the right menu button in MZMenu, this close button is in a
+                    //"Hovered" state, even though it wasn't hovered


Could this be because the mouseup event that ends the hovered state is sent to the help sheet, so the right menu button's screen doesn't receive it and assumes that it is still hovered?

It is the "X" button on the help sheet that is in the hovered state when the right menu button on the source screen is clicked.

Am I misunderstanding your comment?

@vinocher are you asking if the help sheet is "stealing" the mouseup event? Seems plausible.

Yes, I was wondering it the mouse down and up are going to two different targets, causing this issue. I don't think anything needs to be addressed in this comment.

vinocher · 2024-01-02T18:57:58Z

src/mozillavpn.cpp

+    return "https://support.mozilla.org/en-US/kb/"
+           "how-do-i-change-my-dns-settings";


Should these strings be moved into Constants, like the other UrlLabels?

And we just need to drop the "en-US" so that moz.org can route to whichever locale is appropriate.

Co-authored-by: Francesco Lodolo <[email protected]>

lesleyjanenorton · 2024-01-12T22:51:00Z

nebula/ui/components/MZHelpSheet.qml

                    Layout.rightMargin: MZTheme.theme.windowMargin / 2

                    Layout.preferredHeight: MZTheme.theme.rowHeight
                    Layout.preferredWidth: MZTheme.theme.rowHeight

+                    //Hacky workaround because for some reason, when opening a sheet via
+                    //the right menu button in MZMenu, this close button is in a
+                    //"Hovered" state, even though it wasn't hovered


@vinocher are you asking if the help sheet is "stealing" the mouseup event? Seems plausible.

lesleyjanenorton · 2024-01-12T22:53:58Z

src/mozillavpn.cpp

+    return "https://support.mozilla.org/en-US/kb/"
+           "how-do-i-change-my-dns-settings";


And we just need to drop the "en-US" so that moz.org can route to whichever locale is appropriate.

lesleyjanenorton · 2024-01-12T23:00:36Z

src/translations/strings.yaml

+    value: What is custom DNS?
+    comment: Header label for the custom dns help sheet
+  dnsBody1:
+    value: Whenever you connect to a website, a DNS (domain name system) first turns the domain name (e.g. www.mozilla.org) into an IP address that allows your internet traffic to reach that destination.


+1 @vinocher . We don't seem to be answering the question from dnsHeader 'What is a custom DNS?').

…vpn-client into VPN-4687

vinocher

Looks good!

MattLichtenstein requested review from lesleyjanenorton and vinocher December 22, 2023 20:44

MattLichtenstein requested a review from flodolo as a code owner December 22, 2023 20:44

dns settings help sheet

c5d1805

MattLichtenstein force-pushed the VPN-4687 branch from 92a553a to c5d1805 Compare December 22, 2023 21:17

Foxbolts added 8 commits December 22, 2023 16:19

linter

4076d51

slight tweak to helpsheet layout

40dfab7

fix qt 6.4 layout issue with help sheet

25c90e9

fix right menu button margins

2637469

remove unnecessary id

58ecf5b

fix qt 6.4 layout issue again

ce1d69d

consistent icon

89a89bc

syntax

51f5de7

This was referenced Dec 27, 2023

VPN-4691: Add location selection help sheet #8873

Merged

VPN-4685: Add device management help sheet #8876

Merged

VPN-4684: Add privacy features help sheet #8878

Merged

flodolo approved these changes Dec 29, 2023

View reviewed changes

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

MattLichtenstein mentioned this pull request Dec 29, 2023

VPN-4690: Add excluded apps help sheet #8879

Merged

vinocher reviewed Jan 2, 2024

View reviewed changes

Foxbolts and others added 2 commits January 4, 2024 16:54

Update src/translations/strings.yaml

8452b15

Co-authored-by: Francesco Lodolo <[email protected]>

code review changes

5d0c10e

MattLichtenstein requested a review from vinocher January 4, 2024 22:18

lesleyjanenorton requested changes Jan 12, 2024

View reviewed changes

Foxbolts added 2 commits January 16, 2024 13:55

Merge branch 'VPN-4687' of https://github.com/mozilla-mobile/mozilla-…

2f76a5c

…vpn-client into VPN-4687

remove locale from sumo link

731a028

MattLichtenstein requested a review from lesleyjanenorton January 18, 2024 17:14

vinocher approved these changes Jan 18, 2024

View reviewed changes

linter

ed4193a

lesleyjanenorton approved these changes Jan 18, 2024

View reviewed changes

MattLichtenstein merged commit ad299d3 into main Jan 18, 2024
125 checks passed

MattLichtenstein deleted the VPN-4687 branch January 18, 2024 19:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

VPN-4687: Add DNS settings help sheet #8864

VPN-4687: Add DNS settings help sheet #8864

MattLichtenstein commented Dec 22, 2023 •

edited

Loading

flodolo left a comment

vinocher Jan 2, 2024 •

edited

Loading

MattLichtenstein Jan 4, 2024

lesleyjanenorton Jan 12, 2024

traceyvantyghem Jan 18, 2024

vinocher Jan 2, 2024

vinocher Jan 2, 2024

vinocher Jan 2, 2024

MattLichtenstein Jan 4, 2024

vinocher Jan 17, 2024

vinocher Jan 2, 2024

MattLichtenstein Jan 4, 2024

vinocher Jan 2, 2024 •

edited

Loading

MattLichtenstein Jan 4, 2024

lesleyjanenorton Jan 12, 2024

vinocher Jan 16, 2024

vinocher Jan 2, 2024

lesleyjanenorton Jan 12, 2024

lesleyjanenorton Jan 12, 2024

lesleyjanenorton Jan 12, 2024

lesleyjanenorton Jan 12, 2024

vinocher left a comment

		return "https://support.mozilla.org/en-US/kb/"
		"how-do-i-change-my-dns-settings";

VPN-4687: Add DNS settings help sheet #8864

VPN-4687: Add DNS settings help sheet #8864

Conversation

MattLichtenstein commented Dec 22, 2023 • edited Loading

Description

Reference

flodolo left a comment

Choose a reason for hiding this comment

vinocher Jan 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vinocher Jan 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vinocher left a comment

Choose a reason for hiding this comment

MattLichtenstein commented Dec 22, 2023 •

edited

Loading

vinocher Jan 2, 2024 •

edited

Loading

vinocher Jan 2, 2024 •

edited

Loading