<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/117364/">https://git.reviewboard.kde.org/r/117364/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 5th, 2014, 12:33 p.m. UTC, <b>Amarvir Singh</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;">Answers to questions:
>1. What shall we call the setting for the new mode in the settings file? Right now I call it TrainingMode2 which we obviously need to change.
I see you have changed Practice Modes to Methods, and named the new selector as Practice Mode. So you should change everything that was called Practice Mode to Method now, and changed PracticeMode2 to PracticeMode. Another alternative would been choosing a completely different word for the new Training Mode, and keeping the 'PracticeMode' as it is.
>2. How can we improve the visual layout of the training settings? The new mode selector is separated from the other settings by a qwidget which keeps conjugation settings. But this widget is only visible when the conjugation >training mode is chosen. Perhaps we can make the widget not shown when other modes are active? Or shall we move the new mode setting somewhere else?
Either the qwidget should not take the space, ie use setVisible(false) for conjugation's widget (dunno what is being used right now). This way, when the widget is hidden for other modes, the new drop down selector automatically gets shifted upwards.
Or you could place the new Practice Mode between the language selectors and the Practice Method. This might make sense logically as choosing the Practice Mode should be the next step after choosing the language.
Other points:
Some visual stuff: The known language selector's printed statement is in line with "Statistics for xyz". I recognize that it has been this way before you worked on it, too. But, it sort of doesn't work now. Maybe just shift it one line below.
Also the new drop down selector's default widths cause some visual problems when Conjugation Mode is selected. Earlier, the listbox had the same default width as the Conjugation Mode qwidget and hence there was no sudden transition in widths. But now, the selectors' widths are smaller, and hence changing to Conjugation Mode causes a sudden awkward transition in widths.</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;">> I see you have changed Practice Modes to Methods, and named the new selector as Practice Mode.
Yeah, I think I did that before we discussed it and agreed not to do that. Sorry. I can change it back easily.
> Another alternative would been choosing a completely different word for the new Training Mode, and keeping the 'PracticeMode' as it is.
Yes, I think that's what we agreed to do. It's just that we haven't found this new magic word yet. :)
I agree with the rest of your comments.</pre>
<br />
<p>- Inge</p>
<br />
<p>On April 4th, 2014, 3:37 a.m. UTC, Inge Wallin wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDE Edu and Amarvir Singh.</div>
<div>By Inge Wallin.</div>
<p style="color: grey;"><i>Updated April 4, 2014, 3:37 a.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;">This patch implementes dual-direction training, which we have also called mixed mode training. The patch is not ready for merging yet but I want feedback on the solution and there are some things to discuss.
Visually, the patch removes the language selection listbox and instead adds two selectors: "known language" (the user should select the native language if possible) and "language to learn". In addition to that there is also a new mode selector for the type of practice that the user wants: known-to-learning, learning-to-known our dual direction / mixed mode.
In the future I will also implement sound-only to known, which trains the ability to understand spoken language, which is not the same as reading. But I cannot do that yet since the kvtml format doesn't support enough ways to keep the training status (grades).
For this review, I would prefer higher-level comments. I will find all trailing spaces and undone FIXME's myself and fix them before the next time around.
So, the questions then:
1. What shall we call the setting for the new mode in the settings file? Right now I call it TrainingMode2 which we obviously need to change.
2. How can we improve the visual layout of the training settings? The new mode selector is separated from the other settings by a qwidget which keeps conjugation settings. But this widget is only visible when the conjugation training mode is chosen. Perhaps we can make the widget not shown when other modes are active? Or shall we move the new mode setting somewhere else?
Feedback is most welcome.</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;">Tested with flashcards. More testing is also very welcome, especially by people who know how conjugation, comparition and gender training works.</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/statistics/statisticsmainwindow.cpp <span style="color: grey">(24d23bf)</span></li>
<li>src/statistics/statisticsmainwindow.ui <span style="color: grey">(2426fd7)</span></li>
<li>src/statistics/statisticsmainwindow.h <span style="color: grey">(9cbce05)</span></li>
<li>src/settings/parley.kcfg <span style="color: grey">(c831b13)</span></li>
<li>src/practice/writtenbackendmode.h <span style="color: grey">(86138be)</span></li>
<li>src/practice/writtenbackendmode.cpp <span style="color: grey">(2b3bc9b)</span></li>
<li>src/practice/testentry.cpp <span style="color: grey">(9f6b8db)</span></li>
<li>src/practice/sessionmanagerfixed.cpp <span style="color: grey">(18a11a3)</span></li>
<li>src/practice/testentry.h <span style="color: grey">(e779075)</span></li>
<li>src/practice/sessionmanagerbase.cpp <span style="color: grey">(69a1a7d)</span></li>
<li>src/practice/sessionmanagerbase.h <span style="color: grey">(8066279)</span></li>
<li>src/practice/genderbackendmode.h <span style="color: grey">(bf0bf47)</span></li>
<li>src/practice/flashcardbackendmode.cpp <span style="color: grey">(dc0e8e6)</span></li>
<li>src/practice/practicesummarycomponent.cpp <span style="color: grey">(bca5b94)</span></li>
<li>src/practice/practicestatemachine.cpp <span style="color: grey">(8ae77b5)</span></li>
<li>src/practice/practicestatemachine.h <span style="color: grey">(e1dbc5c)</span></li>
<li>src/practice/practiceoptions.cpp <span style="color: grey">(e7ff9aa)</span></li>
<li>src/practice/practicemainwindow.cpp <span style="color: grey">(9be490e)</span></li>
<li>src/practice/practiceoptions.h <span style="color: grey">(b5a1cf7)</span></li>
<li>src/practice/multiplechoicebackendmode.cpp <span style="color: grey">(e2c5132)</span></li>
<li>src/practice/multiplechoicebackendmode.h <span style="color: grey">(5ee9249)</span></li>
<li>src/practice/genderbackendmode.cpp <span style="color: grey">(997f400)</span></li>
<li>src/practice/flashcardbackendmode.h <span style="color: grey">(32f43d2)</span></li>
<li>src/practice/examplesentencebackendmode.cpp <span style="color: grey">(1caa3f6)</span></li>
<li>src/practice/examplesentencebackendmode.h <span style="color: grey">(10d9e72)</span></li>
<li>src/practice/entryfilter.cpp <span style="color: grey">(7a0e759)</span></li>
<li>src/practice/entryfilter.h <span style="color: grey">(c1fd843)</span></li>
<li>src/practice/conjugationbackendmode.cpp <span style="color: grey">(749fb33)</span></li>
<li>src/practice/conjugationbackendmode.h <span style="color: grey">(91da66d)</span></li>
<li>src/practice/comparisonbackendmode.cpp <span style="color: grey">(2bc1c50)</span></li>
<li>src/practice/comparisonbackendmode.h <span style="color: grey">(deeeb28)</span></li>
<li>src/practice/abstractbackendmode.cpp <span style="color: grey">(9a08575)</span></li>
<li>src/practice/abstractbackendmode.h <span style="color: grey">(10e9162)</span></li>
<li>src/practice/CLASSES <span style="color: grey">(941f801)</span></li>
<li>src/parleymainwindow.cpp <span style="color: grey">(74ff823)</span></li>
<li>src/CMakeLists.txt <span style="color: grey">(fb29bc6)</span></li>
<li>TODO-MIXEDMODE <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/117364/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>