<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 7th, 2014, 2:02 p.m. IST, <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;">I've tried understanding the code, but finding issues in the logic of the code gets pretty difficult in such a huge patch. Hence, I trust that you did a good job and I haven't really looked for logical issues. I did an overview of the code and found some minor issues. Also do take care of the FIXME's and the 'reminder' comments before the final revision. :)
Now for the testing:
1. By shifting the left side of the main window down, the "Lesson statistics for xyz file" title is not visible.
2. Comparison mode issue: I'll include the kvtml I've been using, look for the very last "New Lesson" to test it.
The problem can be duplicated in any existing kvtml too by doing the following:
a. Go to editor, make a lesson with just one entry and then view->inflection widget.
b. Select a word in one language, select adjective or adverb in inflection widget, and enter the comparison forms.
c. Don't enter the comparison form for the other language. (this is what causes the problem, if both translations have their comparison forms filled, the mixed mode works fine.)
d. Go to practice, check for learning to known, or known to learning comparison form practice. One of them will work.
e. Now check for the mixed mode. Actual Result: Displays a question for the form not entered, and expects the answer to be blank spaces. Doesn't even show the correctly entered form as a question.
Expected Result: Should not display the unentered forms, and should display the other question for the translation with entered forms.</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;">Check your mail for the test kvtml.</pre>
<br />
<p>- Amarvir</p>
<br />
<p>On April 6th, 2014, 2:43 p.m. IST, 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 6, 2014, 2:43 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;">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>TODO-MIXEDMODE <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/CMakeLists.txt <span style="color: grey">(fb29bc6)</span></li>
<li>src/parleymainwindow.cpp <span style="color: grey">(74ff823)</span></li>
<li>src/practice/CLASSES <span style="color: grey">(941f801)</span></li>
<li>src/practice/abstractbackendmode.h <span style="color: grey">(10e9162)</span></li>
<li>src/practice/abstractbackendmode.cpp <span style="color: grey">(9a08575)</span></li>
<li>src/practice/comparisonbackendmode.h <span style="color: grey">(deeeb28)</span></li>
<li>src/practice/comparisonbackendmode.cpp <span style="color: grey">(2bc1c50)</span></li>
<li>src/practice/conjugationbackendmode.h <span style="color: grey">(91da66d)</span></li>
<li>src/practice/conjugationbackendmode.cpp <span style="color: grey">(749fb33)</span></li>
<li>src/practice/entryfilter.h <span style="color: grey">(c1fd843)</span></li>
<li>src/practice/entryfilter.cpp <span style="color: grey">(7a0e759)</span></li>
<li>src/practice/examplesentencebackendmode.h <span style="color: grey">(10d9e72)</span></li>
<li>src/practice/examplesentencebackendmode.cpp <span style="color: grey">(1caa3f6)</span></li>
<li>src/practice/flashcardbackendmode.h <span style="color: grey">(32f43d2)</span></li>
<li>src/practice/flashcardbackendmode.cpp <span style="color: grey">(dc0e8e6)</span></li>
<li>src/practice/genderbackendmode.h <span style="color: grey">(bf0bf47)</span></li>
<li>src/practice/genderbackendmode.cpp <span style="color: grey">(997f400)</span></li>
<li>src/practice/multiplechoicebackendmode.h <span style="color: grey">(5ee9249)</span></li>
<li>src/practice/multiplechoicebackendmode.cpp <span style="color: grey">(e2c5132)</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/practiceoptions.cpp <span style="color: grey">(e7ff9aa)</span></li>
<li>src/practice/practicestatemachine.h <span style="color: grey">(e1dbc5c)</span></li>
<li>src/practice/practicestatemachine.cpp <span style="color: grey">(8ae77b5)</span></li>
<li>src/practice/practicesummarycomponent.cpp <span style="color: grey">(bca5b94)</span></li>
<li>src/practice/sessionmanagerbase.h <span style="color: grey">(8066279)</span></li>
<li>src/practice/sessionmanagerbase.cpp <span style="color: grey">(69a1a7d)</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/testentry.cpp <span style="color: grey">(9f6b8db)</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/settings/parley.kcfg <span style="color: grey">(c831b13)</span></li>
<li>src/statistics/statisticsmainwindow.h <span style="color: grey">(9cbce05)</span></li>
<li>src/statistics/statisticsmainwindow.cpp <span style="color: grey">(24d23bf)</span></li>
<li>src/statistics/statisticsmainwindow.ui <span style="color: grey">(2426fd7)</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>