[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-edu-devel
Subject: Re: Review Request 128078: Parley: Remember practice direction for each practice mode
From: Albert Astals Cid <aacid () kde ! org>
Date: 2016-10-27 22:24:03
Message-ID: 20161027222403.9507.71591 () mimi ! kde ! org
[Download RAW message or body]
--===============1041193475998287889==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
> On Oct. 13, 2016, 4:25 p.m., Hartmut Riesenbeck wrote:
> > Does someone may have time to review?
> >
> > I use this patch now since more than four month and find it very \
> > usefull for my way of practicing vocabulary. I don't want to miss it. \
> > May someone agree with me.
>
> Albert Astals Cid wrote:
> Not lots of people developing parley lately it seems, anyhow i had a look \
> at the patch and looks sensible from my "I know nothing about this" point \
> of view.
> If noone disagrees I'll commit it on 4 November, if something breaks \
> you'll have to fix it ;)
> Hartmut Riesenbeck wrote:
> Many thanks for your review, Albert.
> For not to bother you with this, I could do the commit. If no objections, \
> I'll take your suggestion and commit on 4. November.
> Besides there is another bugfix in the pipeline \
> (https://git.reviewboard.kde.org/r/128901/). May I proceed it the same \
> way?
Sure, you can do the commit.
- Albert
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128078/#review99982
-----------------------------------------------------------
On June 6, 2016, 7:47 p.m., Hartmut Riesenbeck wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128078/
> -----------------------------------------------------------
>
> (Updated June 6, 2016, 7:47 p.m.)
>
>
> Review request for KDE Edu and KDE Usability.
>
>
> Repository: parley
>
>
> Description
> -------
>
> New check box in StatistcsMainWindow is added. If check box is
> switched on the practice direction for the current practice mode is
> saved and will be restored when this practice mode is selected again.
>
> This is a feature I personally missed. May it is usefull for the public.
>
> Screenshot 1:
> I am not shure if the location for the check box, which is switching
> the feature on and off, is well choosen here in the main window (see
> screenshot01). Or if it's better placed in the practice configure dialog.
>
> Screenshot 2:
> As a reply of Julians comment, I tried to improve the check box text.
> I hope it is better understandable now (please excuse my limited english
> knowledge). This version is ok for minimal changes on the user interface.
> But for me it is not realy satisfyingly.
>
> Screenshot 3:
> Added frame arount the practise direction group. After this the check
> box text can be reduce a little more. Because it's obvious that the
> direction widgets belongs together. But for me the frame is not very
> harmonic to the rest.
>
> Screenshot 4:
> Grouped all practice controlling widgets into frames. This is a really
> big change in the user interface.But it's my favorite.
>
> I redesigned the user interface of the statistics window from the
> perspective of a programmer. This may not confirm with good user
> interface design. So I added the usability group to the rewiers.
> I would really appreciate any comments or suggestions for improvements.
>
> While working on this feature a smal bug was found in configuration
> file saving. File was only saved when document was modified on
> closing the application. Fixed it.
>
> Sorry for the long text. ;-)
>
>
> Diffs
> -----
>
> src/parleydocument.cpp 2f5d3b32ba4462d6b9c917b109e0787cede59513
> src/parleymainwindow.cpp 9fd1b92e88ed2614f4f71579ed0f2a31fca027c3
> src/settings/parley.kcfg 7eac0d27a6d2f5dcb94d1ed5802b7c05b0ce4eb4
> src/statistics/statisticsmainwindow.h \
> f6c87231010f672c0e32aae34345785d16484d75 \
> src/statistics/statisticsmainwindow.cpp \
> 64c7ad64057e74da5ca81b098c0e865610900bb5 \
> src/statistics/statisticsmainwindow.ui \
> 190017cc3ca788248aa468cc90349dde873fa80d
> Diff: https://git.reviewboard.kde.org/r/128078/diff/
>
>
> Testing
> -------
>
>
> File Attachments
> ----------------
>
> screenshot01
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/01/0d19592c-0402-4b94-90e9-c5aa52fe54b6__screenshot01.png
> Changed Check Box Text
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/06/b7fc88be-6e22-4671-9b6b-60e113cc42f1__screenshot02.png
> Frame around Practise Direction
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/06/85ca885d-5f60-46da-a5a1-c892564f9c70__screenshot03.png
> Practice Selection with three Frames
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/06/8e0b5767-da65-4815-9348-097b25da2ffa__screenshot04.png
>
>
> Thanks,
>
> Hartmut Riesenbeck
>
>
--===============1041193475998287889==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 7bit
<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/128078/">https://git.reviewboard.kde.org/r/128078/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; \
padding-left: 10px;"> <p style="margin-top: 0;">On October 13th, 2016, \
4:25 p.m. UTC, <b>Hartmut Riesenbeck</b> wrote:</p> <blockquote \
style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <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;">Does someone may have time to \
review?</p> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">I use this patch now since \
more than four month and find it very usefull for my way of practicing \
vocabulary. I don't want to miss it. May someone agree with me.</p></pre> \
</blockquote>
<p>On October 26th, 2016, 9:33 p.m. UTC, <b>Albert Astals Cid</b> \
wrote:</p> <blockquote style="margin-left: 1em; border-left: 2px solid \
#d0d0d0; padding-left: 10px;"> <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;">Not lots of \
people developing parley lately it seems, anyhow i had a look at the patch \
and looks sensible from my "I know nothing about this" point of view.</p> \
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">If noone disagrees I'll commit it on 4 \
November, if something breaks you'll have to fix it ;)</p></pre> \
</blockquote>
<p>On October 27th, 2016, 10:02 a.m. UTC, <b>Hartmut Riesenbeck</b> \
wrote:</p> <blockquote style="margin-left: 1em; border-left: 2px solid \
#d0d0d0; padding-left: 10px;"> <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;">Many thanks \
for your review, Albert. For not to bother you with this, I could do the \
commit. If no objections, I'll take your suggestion and commit on 4. \
November.</p> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">Besides there is another \
bugfix in the pipeline (https://git.reviewboard.kde.org/r/128901/). May I \
proceed it the same way?</p></pre> </blockquote>
</blockquote>
<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;">Sure, you can do the commit.</p></pre> <br \
/>
<p>- Albert</p>
<br />
<p>On June 6th, 2016, 7:47 p.m. UTC, Hartmut Riesenbeck 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 KDE Edu and KDE Usability.</div>
<div>By Hartmut Riesenbeck.</div>
<p style="color: grey;"><i>Updated June 6, 2016, 7:47 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
parley
</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;">New check box in \
StatistcsMainWindow is added. If check box is switched on the practice \
direction for the current practice mode is saved and will be restored when \
this practice mode is selected again.</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">This is a feature I personally missed. May it is usefull for the \
public.</p> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">Screenshot 1: I am not shure \
if the location for the check box, which is switching the feature on and \
off, is well choosen here in the main window (see screenshot01). Or if \
it's better placed in the practice configure dialog.</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">Screenshot 2: As a reply of Julians comment, I tried to improve \
the check box text. I hope it is better understandable now (please excuse \
my limited english knowledge). This version is ok for minimal changes on \
the user interface. But for me it is not realy satisfyingly.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Screenshot 3: Added frame arount the \
practise direction group. After this the check box text can be reduce a \
little more. Because it's obvious that the direction widgets belongs \
together. But for me the frame is not very harmonic to the rest.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Screenshot 4: Grouped all practice \
controlling widgets into frames. This is a really big change in the user \
interface.But it's my favorite.</p> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">I redesigned \
the user interface of the statistics window from the perspective of a \
programmer. This may not confirm with good user interface design. So I \
added the usability group to the rewiers. I would really appreciate any \
comments or suggestions for improvements.</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">While working on this feature a smal bug was found in \
configuration file saving. File was only saved when document was modified \
on closing the application. Fixed it.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Sorry for the long text. ;-)</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>src/parleydocument.cpp <span style="color: \
grey">(2f5d3b32ba4462d6b9c917b109e0787cede59513)</span></li>
<li>src/parleymainwindow.cpp <span style="color: \
grey">(9fd1b92e88ed2614f4f71579ed0f2a31fca027c3)</span></li>
<li>src/settings/parley.kcfg <span style="color: \
grey">(7eac0d27a6d2f5dcb94d1ed5802b7c05b0ce4eb4)</span></li>
<li>src/statistics/statisticsmainwindow.h <span style="color: \
grey">(f6c87231010f672c0e32aae34345785d16484d75)</span></li>
<li>src/statistics/statisticsmainwindow.cpp <span style="color: \
grey">(64c7ad64057e74da5ca81b098c0e865610900bb5)</span></li>
<li>src/statistics/statisticsmainwindow.ui <span style="color: \
grey">(190017cc3ca788248aa468cc90349dde873fa80d)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/128078/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/2016/06/01/0d19592c-0402-4b94-90e9-c5aa52fe54b6__screenshot01.png">screenshot01</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/06/06/b7fc88be-6e22-4671-9b6b-60e113cc42f1__screenshot02.png">Changed \
Check Box Text</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/06/06/85ca885d-5f60-46da-a5a1-c892564f9c70__screenshot03.png">Frame \
around Practise Direction</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/06/06/8e0b5767-da65-4815-9348-097b25da2ffa__screenshot04.png">Practice \
Selection with three Frames</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============1041193475998287889==--
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic