Expose some more settings in an Advanced dialog
ClosedPublic

Authored by apol on Jul 9 2019, 4:38 PM.

Details

Summary

Was requested by some users who needed these fields and which are already offered by NM.

Test Plan

Changed them, then changed them back.

Diff Detail

Repository
R116 Plasma Network Management Applet
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
apol created this revision.Jul 9 2019, 4:38 PM
Restricted Application added a project: Plasma. · View Herald TranscriptJul 9 2019, 4:38 PM
Restricted Application added 1 blocking reviewer(s): jgrulich. · View Herald Transcript
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
apol requested review of this revision.Jul 9 2019, 4:38 PM
ngraham added a subscriber: ngraham.Jul 9 2019, 5:58 PM
ngraham added inline comments.
libs/editor/settings/ipv4widget.cpp
439

Add a colon and don't use Title Case: "Send hostname:"

443

Add a colon and don't use Title Case: "DHCP hostname:"

448

Add a colon and don't use Title Case: "DAD timeout:"

broulik added a subscriber: broulik.Jul 9 2019, 8:02 PM
broulik added inline comments.
libs/editor/settings/ipv4widget.cpp
432

What's that QPointer for?
You might want to dlg->setAttribute(Qt::WA_DeleteOnClose);

435

I would prefer that you used a UI file for all of this

463

Doesn't seem to work? Or did you disable the dimming effect or is that a quirk in conjunction with QQuickWidget in systemsettings?

apol marked 4 inline comments as done.Jul 9 2019, 10:52 PM
apol added inline comments.
libs/editor/settings/ipv4widget.cpp
463

Worked for me, I've seen issues with the list view on the left though. :/

apol updated this revision to Diff 61474.Jul 9 2019, 10:52 PM

Addressed comments

Don't you miss loading/saving of those properties in loadConfig() and setting() methods?

I think this UI is missing hints/tips about meaning of these options. Otherwise it would be unclear:

  • What is the meaning of dad-timeout = -1
  • What is the meaning of dad-timeout = 0
  • What happens if dhcp-send-hostname is true, but dhcp-hostname is empty
  • What happens if dhcp-send-hostname is false, but dhcp-hostname is not empty (if it would be ignored, consider disabling the line edit)
apol updated this revision to Diff 61510.Jul 10 2019, 1:46 PM

addressed comments

You still need to add following lines to the loadConfig() method, otherwise those properties won't be filled when editing an existing connection:

m_tmpIpv4Setting.setDhcpHostname(ipv4Setting->dhcpHostname());
m_tmpIpv4Setting.setDhcpSendHostname(ipv4Setting->dhcpSendHostname());
m_tmpIpv4Setting.setDadTimeout(ipv4Setting->dadTimeout());
apol updated this revision to Diff 61520.Jul 10 2019, 3:32 PM

add loadConfig bits

jgrulich accepted this revision.Jul 11 2019, 5:57 AM

Looks good to me now.

This revision is now accepted and ready to land.Jul 11 2019, 5:57 AM
This revision was automatically updated to reflect the committed changes.
broulik added inline comments.Jul 11 2019, 9:56 AM
libs/editor/settings/ipv4widget.cpp
451

a specialValueText() for -1 "Default" would have been lovely

460

Missing a this context