<table><tr><td style="">pino added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D15093">View Revision</a></tr></table><br /><div><div><blockquote style="border-left: 3px solid #8C98B8;
color: #6B748C;
font-style: italic;
margin: 4px 0 12px 0;
padding: 8px 12px;
background-color: #F8F9FC;">
<div style="font-style: normal;
padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D15093#317960" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">D15093#317960</a>, <a href="https://phabricator.kde.org/p/andersonbruce/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@andersonbruce</a> wrote:</div>
<div style="margin: 0;
padding: 0;
border: 0;
color: rgb(107, 116, 140);"><p>I've used KDE for years but this is the first time I've written code using Qt so it doesn't surprise me that I didn't use some of the preferred methods of doing things. I have a few questions below and hopefully you will have a little patience with me if any seem like stupid questions.</p>
<blockquote style="border-left: 3px solid #8C98B8;
color: #6B748C;
font-style: italic;
margin: 4px 0 12px 0;
padding: 8px 12px;
background-color: #F8F9FC;">
<div style="font-style: normal;
padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D15093#315890" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">D15093#315890</a>, <a href="https://phabricator.kde.org/p/pino/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@pino</a> wrote:</div>
<div style="margin: 0;
padding: 0;
border: 0;
color: rgb(107, 116, 140);"><p>Misc notes:</p>
<ul class="remarkup-list">
<li class="remarkup-list-item">please remove all the translations (i.e. <tt style="background: #ebebeb; font-size: 13px;">Name=[lang]</tt> & <tt style="background: #ebebeb; font-size: 13px;">Comment[lang]</tt> from desktop/json files, since KDE has an automatic system to handle them</li>
</ul></div>
</blockquote>
<p>Do the automatic translations get added into the desktop files themselves at some point of the build process?</p></div>
</blockquote>
<p>There is an automatic system that extracts translations from desktop files (and alike), and injects translations back.</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><ul class="remarkup-list">
<li class="remarkup-list-item">as <a href="https://phabricator.kde.org/p/lbeltrame/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@lbeltrame</a> said, hardcoding colors is a bad idea, and it will not play nice with user choice of different color schemes, or accessibility purposes</li>
</ul></blockquote>
<p>What I am trying to do here is to highlight fields that don't have valid entries in them. I did this by turning the background in the field red if it wasn't valid and returning it to default when it became valid. Is there a "KDE way" to do something like this or should I just leave everything with the default background and not give the user any immediate feedback that there is a problem?</p></blockquote>
<p>I mentioned it one point below:</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><ul class="remarkup-list">
<li class="remarkup-list-item">a better option to validate an input in a line edit is to use the embedded validator options, see <tt style="background: #ebebeb; font-size: 13px;">QLineEdit::setInputMask</tt> and <tt style="background: #ebebeb; font-size: 13px;">QValidator</tt></li>
</ul></blockquote></blockquote>
<p>IOW, instead of validating the input <em>after</em> the user inputs it, do it <em>before</em>.</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><ul class="remarkup-list">
<li class="remarkup-list-item">the better option to edit a port number is <tt style="background: #ebebeb; font-size: 13px;">QSpinBox</tt> restricted to 0-65536, instead of a line edit</li>
</ul></blockquote>
<p>For the one entry which specifies only a port number, the most common instance is to leave this field empty. In my quick read through of the QSpinBox docs I didn't see any way to do a mixed 'no entry'/number option so I will probably leave this as a line edit.</p></blockquote>
<p>IMHO having 0 as value for that is good; otherwise, a proper QIntValidator for the line edit restricts the input the user can insert, removing the need to do the validation manually.</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><ul class="remarkup-list">
<li class="remarkup-list-item">there is a mixture of <tt style="background: #ebebeb; font-size: 13px;">virtual</tt> & <tt style="background: #ebebeb; font-size: 13px;">Q_DECL_OVERRIDE</tt> for virtual methods; plasma-nm uses <tt style="background: #ebebeb; font-size: 13px;">override</tt> exclusively</li>
</ul></blockquote>
<p>Can you please give explicit references to where the problem is?</p></blockquote>
<p><tt style="background: #ebebeb; font-size: 13px;">wireguardwidget.h</tt>, for example.</p>
<p>I am not familiar with how Q_DECL_OVERRIDE is used but I did look at how all the other VPN implementations did it and it looks like they use the exact same mix of <tt style="background: #ebebeb; font-size: 13px;">virtual</tt> & <tt style="background: #ebebeb; font-size: 13px;">Q_DECL_OVERRIDE</tt> as I used in WireGuard.</p>
<p>See commit <a href="https://phabricator.kde.org/R116:111ac65ae79f1a777e3b4a6389e916f0dfccd35e" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">111ac65ae79f1a777e3b4a6389e916f0dfccd35e</a>. It looks like you are developing against an old version of plasma-nm, or not on master anyway.</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><ul class="remarkup-list">
<li class="remarkup-list-item">please do not hardcode sizes and fonts in UI files</li>
</ul></blockquote>
<p>I can understand that fonts shouldn't be specified and have removed them. What I was trying to do was to highlight the text in a couple of labels by making them a little bigger than the default font and making them bold. Is there a "KDE way" to highlight something like this? Maybe a way to say "this is a header" or similar?</p></blockquote>
<p>Rather than "how can I make some text bigger", the question is "what do you need to do". Since you are grouping widgets, then use a QGroupBox.</p>
<p>Oh, and that adds another thing to fix: <tt style="background: #ebebeb; font-size: 13px;">wireguard.ui</tt> really needs a top-level layout, instead of hardcoding the size of the widgets...</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><ul class="remarkup-list">
<li class="remarkup-list-item">there are various texts in UI files with double spaces between words, please simply them to a single space</li>
<li class="remarkup-list-item">manually calling <tt style="background: #ebebeb; font-size: 13px;">KAcceleratorManager::manage(this);</tt> is not needed, why were they added?</li>
</ul></blockquote>
<p>Again this is due to my inexperience with Qt. Is there some reason that WireGuard doesn't need this but all the other VPN implementations do?</p></blockquote>
<p>This was added by <a href="https://phabricator.kde.org/R116:dbb4e2d5d47a6546014d433a1533d4ef4cfb7137" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">dbb4e2d5d47a6546014d433a1533d4ef4cfb7137</a>. Weird choice, I guess this can be left as it is.</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R116 Plasma Network Management Applet</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D15093">https://phabricator.kde.org/D15093</a></div></div><br /><div><strong>To: </strong>andersonbruce, Plasma, jgrulich, pino<br /><strong>Cc: </strong>acrouthamel, K900, anthonyfieroni, pino, lbeltrame, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart<br /></div>