[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-panel-devel
Subject: Re: Review Request 120002: Systemloadviewer: settings window redesign
From: "David Edmundson" <david () davidedmundson ! co ! uk>
Date: 2014-08-30 12:56:55
Message-ID: 20140830125655.14966.41650 () probe ! kde ! org
[Download RAW message or body]
--===============6218249649968574588==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
> On Aug. 30, 2014, 12:41 p.m., Martin Yrjölä wrote:
> > applets/systemloadviewer/package/contents/ui/ColorSettings.qml, lines 49-50
> > <https://git.reviewboard.kde.org/r/120002/diff/1/?file=308582#file308582line49>
> >
> > Is anchors the correct way to implement spacing in QML? The HIG states in [1] \
> > that the sub-options should be indented by using a horizontal spacer of SizeType \
> > "Minimum". What would be the QML equivalent?
> > [1] https://techbase.kde.org/Projects/Usability/HIG/Form_Label_Alignment#Aligning_Sub-Options
> >
yeah, that's right.
> On Aug. 30, 2014, 12:41 p.m., Martin Yrjölä wrote:
> > applets/systemloadviewer/package/contents/ui/GeneralSettings.qml, lines 74-78
> > <https://git.reviewboard.kde.org/r/120002/diff/1/?file=308583#file308583line74>
> >
> > Can I get rid of this Item somehow? I'm aiming for approach 1 in the checkbox \
> > alignment guidelines [1]. If I put the rowSpan in the checkbox group's label then \
> > the label gets vertically centered in the middle of the group. But if I then \
> > apply `Layout.alignment: Qt.AlignTop` the labels doesn't line up correctly. Same \
> > problem on line 91-94.
> > [1] https://techbase.kde.org/Projects/Usability/HIG/Form_Label_Alignment#Checkboxes
> >
ah. I can see why that could be an issue.
I'll have a play for a few minutes, worst case we keep what we have.
> On Aug. 30, 2014, 12:41 p.m., Martin Yrjölä wrote:
> > applets/systemloadviewer/package/contents/ui/GeneralSettings.qml, line 154
> > <https://git.reviewboard.kde.org/r/120002/diff/1/?file=308583#file308583line154>
> >
> > Is the i18n call needed for a single character abbreviation?
It needs to be i18n. The chinese abbreviation for "seconds" is unlikely to be "s".
however, equally translators aren't going to understand how to translate "s". So we \
need to tell them what it is, there's a call i18nc where you can give translators a \
sentence to give them some context.
i18nc("Abbreviation for secounds", "s");
- David
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120002/#review65538
-----------------------------------------------------------
On Aug. 30, 2014, 12:20 p.m., Martin Yrjölä wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120002/
> -----------------------------------------------------------
>
> (Updated Aug. 30, 2014, 12:20 p.m.)
>
>
> Review request for Plasma and David Edmundson.
>
>
> Repository: kdeplasma-addons
>
>
> Description
> -------
>
> More polish by courtesy of the VDG.
>
>
> Diffs
> -----
>
> applets/systemloadviewer/package/contents/ui/ColorPicker.qml \
> 1b3c88233da75b9d001a0159dd4298d41f7d307c \
> applets/systemloadviewer/package/contents/ui/ColorSettings.qml PRE-CREATION \
> applets/systemloadviewer/package/contents/ui/GeneralSettings.qml \
> fd719d5f2afed8c630fd6a1281f6ddda674b60c2 \
> applets/systemloadviewer/package/contents/ui/SystemLoadViewer.qml \
> 9e101f8fd68602495366c0dc8226927a572e0267 \
> applets/systemloadviewer/package/contents/config/config.qml \
> 570cc77ece3ed0bf84d89e45426773954f95d409 \
> applets/systemloadviewer/package/contents/config/main.xml \
> 5e38045eb369e029564abf973827eb84534eb9f4
> Diff: https://git.reviewboard.kde.org/r/120002/diff/
>
>
> Testing
> -------
>
> Mostly cosmetic changes, but I tested that the options does still work.
>
>
> File Attachments
> ----------------
>
> General settings
> https://git.reviewboard.kde.org/media/uploaded/files/2014/08/30/7ed9b718-93bd-4936-98dd-7a58b8b51128__2014-08-30-142153_361x169_scrot.png
> Color settings with enabled manual color settings
> https://git.reviewboard.kde.org/media/uploaded/files/2014/08/30/8e2367fd-9f44-4bd2-87fe-df5ffbd746be__2014-08-30-143838_256x304_scrot.png
> Color settings with disabled manual color settings
> https://git.reviewboard.kde.org/media/uploaded/files/2014/08/30/8399a4e1-7fa0-4369-ae30-cadbe455d2bb__2014-08-30-143853_254x304_scrot.png
>
>
> Thanks,
>
> Martin Yrjölä
>
>
--===============6218249649968574588==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 8bit
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 \
solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;"> \
<tr> <td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/120002/">https://git.reviewboard.kde.org/r/120002/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <p style="margin-top: 0;">On August 30th, 2014, 12:41 p.m. UTC, <b>Martin \
Yrjölä</b> wrote:</p> <blockquote style="margin-left: 1em; border-left: 2px solid \
#d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;"> <a \
href="https://git.reviewboard.kde.org/r/120002/diff/1/?file=308582#file308582line49" \
style="color: black; font-weight: bold; text-decoration: \
underline;">applets/systemloadviewer/package/contents/ui/ColorSettings.qml</a> <span \
style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th> <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td> <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">49</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> \
<span class="k">anchors.left:</span> <span class="nx">parent</span><span \
class="p">.</span><span class="nx">left</span></pre></td> </tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th> <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td> <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">50</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> \
<span class="k">anchors.leftMargin:</span> <span class="nx">units</span><span \
class="p">.</span><span class="nx">largeSpacing</span></pre></td> </tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Is \
anchors the correct way to implement spacing in QML? The HIG states in [1] that the \
sub-options should be indented by using a horizontal spacer of SizeType "Minimum". \
What would be the QML equivalent?</p> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">[1] \
https://techbase.kde.org/Projects/Usability/HIG/Form_Label_Alignment#Aligning_Sub-Options</p></pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">yeah, that's right.</p></pre> <br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <p style="margin-top: 0;">On August 30th, 2014, 12:41 p.m. UTC, <b>Martin \
Yrjölä</b> wrote:</p> <blockquote style="margin-left: 1em; border-left: 2px solid \
#d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;"> <a \
href="https://git.reviewboard.kde.org/r/120002/diff/1/?file=308583#file308583line74" \
style="color: black; font-weight: bold; text-decoration: \
underline;">applets/systemloadviewer/package/contents/ui/GeneralSettings.qml</a> \
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2">74</font></th> <td bgcolor="#fdfebc" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="nx"><span \
class="hl">Label</span></span> <span class="p">{</span></pre></td> <th \
bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">63</font></th> <td bgcolor="#fdfebc" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="nx"><span class="hl">Item</span></span> <span class="p">{</span></pre></td> \
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2">75</font></th> <td bgcolor="#fdfebc" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="k">text:</span> <span class="nx">i18n</span><span class="p">(</span><span \
class="s2">"Use theme colors:"</span><span class="p">)</span></pre></td> \
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">64</font></th> <td bgcolor="#fdfebc" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> \
<span class="k">width:</span> <span class="mi">1</span></pre></td> </tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2">76</font></th> <td bgcolor="#fdfebc" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="k">Layout.alignment:</span> <span class="nx">Qt</span><span \
class="p">.</span><span class="nx">AlignRight</span></pre></td> <th \
bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">65</font></th> <td bgcolor="#fdfebc" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> \
<span class="k">height:</span> <span class="mi">1</span></pre></td> </tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th> <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td> <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">66</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> \
<span class="k">Layout.rowSpan:</span> <span class="mi">2</span></pre></td> </tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2">77</font></th> <td bgcolor="#ffffff" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="p">}</span></pre></td> <th bgcolor="#f0f0f0" style="border-left: 1px solid \
#C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font \
size="2">67</font></th> <td bgcolor="#ffffff" width="50%"><pre style="font-size: \
8pt; line-height: 140%; margin: 0; "> <span class="p">}</span></pre></td> \
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Can I \
get rid of this Item somehow? I'm aiming for approach 1 in the checkbox alignment \
guidelines [1]. If I put the rowSpan in the checkbox group's label then the label \
gets vertically centered in the middle of the group. But if I then apply <code \
style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: \
0;line-height: inherit;">Layout.alignment: Qt.AlignTop</code> the labels doesn't line \
up correctly. Same problem on line 91-94.</p> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">[1] \
https://techbase.kde.org/Projects/Usability/HIG/Form_Label_Alignment#Checkboxes</p></pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">ah. I can see why that could be an issue.<br style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" /> \
I'll have a play for a few minutes, worst case we keep what we have.</p></pre> <br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <p style="margin-top: 0;">On August 30th, 2014, 12:41 p.m. UTC, <b>Martin \
Yrjölä</b> wrote:</p> <blockquote style="margin-left: 1em; border-left: 2px solid \
#d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;"> <a \
href="https://git.reviewboard.kde.org/r/120002/diff/1/?file=308583#file308583line154" \
style="color: black; font-weight: bold; text-decoration: \
underline;">applets/systemloadviewer/package/contents/ui/GeneralSettings.qml</a> \
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2">148</font></th> <td bgcolor="#fdfebc" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="nx">Label</span> <span class="p">{</span></pre></td> <th bgcolor="#e9eaa8" \
style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" \
align="right"><font size="2">113</font></th> <td bgcolor="#fdfebc" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="k">suffix:</span> <span class="nx">i18n</span><span class="p">(</span><span \
class="s2">"s"</span><span class="p">)</span></pre></td> </tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Is \
the i18n call needed for a single character abbreviation?</p></pre> </blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">It needs to be i18n. The chinese abbreviation for "seconds" is unlikely to \
be "s".</p> <p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">however, equally translators aren't going to \
understand how to translate "s". So we need to tell them what it is, there's a call \
i18nc where you can give translators a sentence to give them some context.</p> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">i18nc("Abbreviation for secounds", "s");</p></pre> <br />
<p>- David</p>
<br />
<p>On August 30th, 2014, 12:20 p.m. UTC, Martin Yrjölä wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: \
1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; \
-webkit-border-radius: 6px;"> <tr>
<td>
<div>Review request for Plasma and David Edmundson.</div>
<div>By Martin Yrjölä.</div>
<p style="color: grey;"><i>Updated Aug. 30, 2014, 12:20 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdeplasma-addons
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">More polish by courtesy of the VDG.</p></pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Mostly cosmetic changes, but I tested that the options \
does still work.</p></pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>applets/systemloadviewer/package/contents/ui/ColorPicker.qml <span style="color: \
grey">(1b3c88233da75b9d001a0159dd4298d41f7d307c)</span></li>
<li>applets/systemloadviewer/package/contents/ui/ColorSettings.qml <span \
style="color: grey">(PRE-CREATION)</span></li>
<li>applets/systemloadviewer/package/contents/ui/GeneralSettings.qml <span \
style="color: grey">(fd719d5f2afed8c630fd6a1281f6ddda674b60c2)</span></li>
<li>applets/systemloadviewer/package/contents/ui/SystemLoadViewer.qml <span \
style="color: grey">(9e101f8fd68602495366c0dc8226927a572e0267)</span></li>
<li>applets/systemloadviewer/package/contents/config/config.qml <span style="color: \
grey">(570cc77ece3ed0bf84d89e45426773954f95d409)</span></li>
<li>applets/systemloadviewer/package/contents/config/main.xml <span style="color: \
grey">(5e38045eb369e029564abf973827eb84534eb9f4)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/120002/diff/" style="margin-left: \
3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments \
</h1>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/08/30/7ed9b718-93bd-4936-98dd-7a58b8b51128__2014-08-30-142153_361x169_scrot.png">General \
settings</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/08/30/8e2367fd-9f44-4bd2-87fe-df5ffbd746be__2014-08-30-143838_256x304_scrot.png">Color \
settings with enabled manual color settings</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/08/30/8399a4e1-7fa0-4369-ae30-cadbe455d2bb__2014-08-30-143853_254x304_scrot.png">Color \
settings with disabled manual color settings</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============6218249649968574588==--
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic