Add daemon restart button to split tunneling settings on MacOS

[hulthe]

---

This change is 

[linear]

DES-1453 Add "Restart Daemon" button to split-tunneling menu

[hulthe]

Reviewable status: 0 of 6 files reviewed, 1 unresolved discussion

---

desktop/packages/mullvad-vpn/src/renderer/components/SplitTunnelingSettings.tsx line 563 at r1 (raw file):

            )}
          </StyledMiniTitle>
          <Spacing height="8px" />

I'm not crazy about adding the Spacing component. Having the spacing expressed as a marginTop in the individual components didn't feel right either though.

[raksooo]

Reviewed 4 of 6 files at r1, all commit messages.
Reviewable status: 4 of 6 files reviewed, 1 unresolved discussion (waiting on @olmoh)

---

desktop/packages/mullvad-vpn/src/renderer/components/SplitTunnelingSettings.tsx line 563 at r1 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

I'm not crazy about adding the Spacing component. Having the spacing expressed as a marginTop in the individual components didn't feel right either though.

I prefer the marginTop :) Maybe ask Oliver for input? @olmoh.

If all elements should have the same spacing then something like flex gap would be useful but I'm not sure tihs is the case here.

[olmoh]

Reviewed 2 of 6 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @hulthe and @raksooo)

---

desktop/packages/mullvad-vpn/src/renderer/components/SplitTunnelingSettings.tsx line 563 at r1 (raw file):

Previously, raksooo (Oskar Nyberg) wrote…

I prefer the marginTop :) Maybe ask Oliver for input? @olmoh.

If all elements should have the same spacing then something like flex gap would be useful but I'm not sure tihs is the case here.

I have nothing against a <Spacing> component, but in that case I think we should have a general one in Layout.tsx that makes use of the spacing tokens and can be reused.
I think in general though what tends to result in the is to not let individual elements handle their spacing, but instead let that be handled by their parents. So I would probably default to using wrapper elements with flex-gap here.

---

desktop/packages/mullvad-vpn/src/renderer/components/SplitTunnelingSettings.tsx line 560 at r1 (raw file):

            {messages.pgettext(
              'split-tunneling-view',
              'Enabled "Fill disk access" and still having issues?',

Should probably be "Full disk access"

---

desktop/packages/mullvad-vpn/src/renderer/components/SplitTunnelingSettings.tsx line 565 at r1 (raw file):

          <Spacing height="8px" />
          <StyledSystemSettingsButton onClick={restartDaemon}>
            Restart Mullvad Service

Should this be localized as well?

[hulthe]

Reviewable status: 5 of 6 files reviewed, 3 unresolved discussions (waiting on @olmoh and @raksooo)

---

desktop/packages/mullvad-vpn/src/renderer/components/SplitTunnelingSettings.tsx line 560 at r1 (raw file):

Previously, olmoh (Oliver Mohlin) wrote…

Should probably be "Full disk access"

Oops. Nice catch.

---

desktop/packages/mullvad-vpn/src/renderer/components/SplitTunnelingSettings.tsx line 563 at r1 (raw file):

Previously, olmoh (Oliver Mohlin) wrote…

I have nothing against a <Spacing> component, but in that case I think we should have a general one in Layout.tsx that makes use of the spacing tokens and can be reused.
I think in general though what tends to result in the is to not let individual elements handle their spacing, but instead let that be handled by their parents. So I would probably default to using wrapper elements with flex-gap here.

Can we use flex-gap here? Given all the spacing is different for each child?

---

desktop/packages/mullvad-vpn/src/renderer/components/SplitTunnelingSettings.tsx line 565 at r1 (raw file):

Previously, olmoh (Oliver Mohlin) wrote…

Should this be localized as well?

Oh yeah I guess it should. "Open System Settings" isn't localized either though. @raksooo?

[olmoh]

Reviewable status: 5 of 6 files reviewed, 2 unresolved discussions (waiting on @hulthe and @raksooo)

---

desktop/packages/mullvad-vpn/src/renderer/components/SplitTunnelingSettings.tsx line 563 at r1 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

Can we use flex-gap here? Given all the spacing is different for each child?

You would need to use different wrappers with different gaps on them :)

[raksooo]

Reviewed 1 of 6 files at r1.
Reviewable status: 5 of 6 files reviewed, 2 unresolved discussions (waiting on @hulthe and @olmoh)

---

desktop/packages/mullvad-vpn/src/renderer/components/SplitTunnelingSettings.tsx line 565 at r1 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

Oh yeah I guess it should. "Open System Settings" isn't localized either though. @raksooo?

Both should be :) We should probably rename StyledSystemSettingsButton to something that describes both.

[hulthe]

Reviewable status: 2 of 8 files reviewed, 2 unresolved discussions (waiting on @olmoh and @raksooo)

---

desktop/packages/mullvad-vpn/src/renderer/components/SplitTunnelingSettings.tsx line 563 at r1 (raw file):

Previously, olmoh (Oliver Mohlin) wrote…

You would need to use different wrappers with different gaps on them :)

I moved Spacing to Layout.tsx, but if using a wrapper with flex-gap is more ideomatic, i can do that instead

---

desktop/packages/mullvad-vpn/src/renderer/components/SplitTunnelingSettings.tsx line 565 at r1 (raw file):

Previously, raksooo (Oskar Nyberg) wrote…

Both should be :) We should probably rename StyledSystemSettingsButton to something that describes both.

Naming is hard, suggestions welcome 🙃

[olmoh]

Reviewed 5 of 6 files at r3.
Reviewable status: 7 of 8 files reviewed, 1 unresolved discussion (waiting on @hulthe and @raksooo)

---

desktop/packages/mullvad-vpn/src/renderer/components/SplitTunnelingSettings.tsx line 563 at r1 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

I moved Spacing to Layout.tsx, but if using a wrapper with flex-gap is more ideomatic, i can do that instead

I think both approaches are generally fine, we are doing some work with spacings parallel with this and flex-gap is more in line with what we are doing there. But I think this is fine for now.

[olmoh]

Reviewed 1 of 6 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved

[Serock3]

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved