Added admin mode to question and answer activity
Needs ReviewPublic

Authored by AkshayCHD on Jan 13 2019, 6:28 PM.

Diff Detail

Repository
R2 GCompris
Lint
Lint Skipped
Unit
Unit Tests Skipped
AkshayCHD created this revision.Jan 13 2019, 6:28 PM
Restricted Application added a project: KDE Edu. · View Herald TranscriptJan 13 2019, 6:28 PM
Restricted Application added a subscriber: kde-edu. · View Herald Transcript
AkshayCHD updated this revision to Diff 49659.Jan 16 2019, 5:47 PM

@echarruau in response to your query on IRC, before making the diff I did a rebase with the latest master branch so the current diff is in sync with the master branch, you can have a look, the code might need some refactoring but, before that i just wanted to have an idea whether i am going in the right direction or not. :)

Hi,
I don't know how much work you have done on it, if it comes from Amit or you but the activity seems much nicer than the last time I saw it.
I saw a few points to improve, even if I did not have a look to the code yet.

  • When I reduce the size of GCompris the equal sign (=) goes under the numbers. You could work on font size to avoid this but in any case the = sign needs to stay on the same line
  • In the admin config menu, numbers are too far from their select box and I did not know if the select box was assigned to the right or the left numbers
  • Admin config: could you increase the fonts
  • Admin config: when reducing some part of the questions disappear, you can rearange then with a flow

-Main window: if the display is too small to display all the questions, the up and down blue arrows used in the config window should appear.

Dataset.js must be placed in ressource directory (see activity categorisation to have an example). First we can build it with a qml file like in categorisation, could become a json later on.

This activity needs to be very generic, questions could be similar to the following one "What is the name of the french capital?" and the activity needs to be able to cope with the position of the quetion and the answer and the size of the answer (answer being given in the qml file we can adapt the size of the answer area).

Good luck :)

AkshayCHD updated this revision to Diff 50096.Jan 23 2019, 11:43 AM

Made the questions orientation more dynamic to support questions of all lengths.

@echarruau I have updated the diff, made the changes as you told, also i've added a sample questions subactivity that would later be deleted just to demonstrate how the questions of different lengths can be accommodated in the activity.

jjazeix added inline comments.
src/activities/question_and_answer/ActivityInfo.qml
28

missing qsTr() (no only there, please fix them everywhere)

src/activities/question_and_answer/Question_and_answer.qml
45

missing dialogActivityConfig.getInitialConfiguration() to initialize the config dialog

69

if it is a string, use a string (else keep it as var)

217

it seems you never actually store the parameters in the config file, so if you do changes, quit the game and restart it, everything is lost?
There is three methods in the item that should be used:
onLoadData, onSaveData and you should set the default values via setDefaultValues instead of doing everything in the onConfigClicked signal.

235

if you use an ExclusiveGroup, you won't need to change the other CheckBox

270

use GCText, not Text to have the same font everywhere

src/activities/question_and_answer/dataset.js
25 ↗(On Diff #50096)

qsTr missing?

src/activities/question_and_answer/question_and_answer.js
140

note that "ii" is smaller than "m" but contains more characters, I'm not sure counting the length of the word is the best way to compare "visual" length

AkshayCHD updated this revision to Diff 50221.Jan 25 2019, 9:46 AM

Made the changes

@jjazeix regarding the part of calculating the longest string, I think only a rough idea of length is needed, it doesn't need to be exact, another option can be to apply that text to the sample question and make comparison on the basis its height, what do you say?

AkshayCHD updated this revision to Diff 50380.Jan 27 2019, 1:45 PM

@jjazeix regarding the part of calculating the longest string, I think only a rough idea of length is needed, it doesn't need to be exact, another option can be to apply that text to the sample question and make comparison on the basis its height, what do you say?

Yes, for now, we can keep it like this, we'll just need to check later if it is still good enough or if we need to be more precise.

AkshayCHD updated this revision to Diff 50617.Jan 31 2019, 8:07 PM

Moved dataset to resource folder in json format.

@jjazeix I have converted the dataset of this diff also to json format

jjazeix added inline comments.Feb 4 2019, 8:07 PM
src/activities/question_and_answer/Question_and_answer.qml
65

int?

72

int?

74

int?

88

you haven't

243

unused?

375

you can use of temp variable to store the children, it will be easier to read after

src/activities/question_and_answer/question_and_answer.js
146

longestText is a better name

I applied your diff and get this answer:
Script qrc:/gcompris/src/activities/question_and_answer/dataset.js unavailable
qrc:/gcompris/src/activities/question_and_answer/dataset.js: No such file or directory
Can you update the diff please?

I think you added an hard coded value for noOfQuestions, but I can not test since I do not have the js at the moment.

src/activities/question_and_answer/Question_and_answer.qml
64

Seems like a hard coded value?

AkshayCHD updated this revision to Diff 50901.Feb 5 2019, 2:41 AM

Made the necessary changes

I applied your diff and get this answer:
Script qrc:/gcompris/src/activities/question_and_answer/dataset.js unavailable
qrc:/gcompris/src/activities/question_and_answer/dataset.js: No such file or directory
Can you update the diff please?

@echarruau you might have to build the project again using cmake as, i have deleted the dataset.js file and added the dataset.json file in its place, so if you run make command then the project would be expecting a file named dataset.js which is no linger there

@jjazeix I have updated the diff

dataset.js is still included in the code.

src/activities/question_and_answer/Question_and_answer.qml
30

You need to remove this line, otherwise it does not compile.

Dataset missing

src/activities/question_and_answer/Question_and_answer.qml
34

I get the following message when running
qrc:/gcompris/src/activities/question_and_answer/Question_and_answer.qml:33: ReferenceError: Dataset is not defined

Buttons start and restart overlapped by menu bar
https://snag.gy/XqvDzO.jpg

I applied your diff and get this answer:
Script qrc:/gcompris/src/activities/question_and_answer/dataset.js unavailable
qrc:/gcompris/src/activities/question_and_answer/dataset.js: No such file or directory
Can you update the diff please?

@echarruau you might have to build the project again using cmake as, i have deleted the dataset.js file and added the dataset.json file in its place, so if you run make command then the project would be expecting a file named dataset.js which is no linger there

Warning message does not fit in the screen
https://snag.gy/a7Y8K1.jpg

AkshayCHD updated this revision to Diff 51033.Feb 6 2019, 1:55 PM
AkshayCHD marked 17 inline comments as done.Feb 6 2019, 2:03 PM
AkshayCHD marked an inline comment as not done.
AkshayCHD added inline comments.
src/activities/question_and_answer/Question_and_answer.qml
64

No these values change depending on the number of questions in the json

AkshayCHD marked an inline comment as not done.Feb 6 2019, 2:05 PM
jjazeix added inline comments.Feb 6 2019, 2:09 PM
src/activities/question_and_answer/resource/dataset.json
1 ↗(On Diff #51033)

The issue that Emmanuel is saying is, (as I understand it), is that we only have one dataset.json file for all the categories.
It would be better to use one qml file (to be able to use qsTr()) per category and "dynamically" load the categories instead of having to update the dataset.json.
This would allow to create dataset independantly of the software compilation and be able to use them without having to recompile (we could say to load all qml dataset files in ~/.local/gcompris... for example).

Could you do a test please?
Remove the start button (only comment it in the code do not delete the lines).
Start should be done when user clicks the first time in any answer box.
Could you also add an item of you choice saying that time is running?
Something nice not stressful. You can look at how candies are animated in share activity to see how you can make objects moving slightly.

Could you work the way questions and answers are displayed?
https://snag.gy/IiCuYd.jpg
We should try to have one question and answer per line, and if there is not enough place for a question in a single line you force the answer under the question.

AkshayCHD updated this revision to Diff 51388.Feb 11 2019, 5:21 AM

Made the changes

Could you please detail what you have done when submitting the final sum up.
Instead of writting "Made the changes" I need to know what has been changed.

I asked you to remove the start button, add a timer, split the json file (which should be several qml files in fact).
I can not see any of these changes. If you write precisely what you have done I will know if I made git apply mistake or if you forgot something.

You need to play with the window size when you work with sizes.
I reduced my screen at the maximum and the text does not fit in the screen.

https://snag.gy/ybjFuB.jpg

Nice improvements on the questions sizes.
Now that this step is started we need to find a way to differentiate short questions (like multiplications) that need to be displayed like it is now displayed, and sentences that need to be displayed in a smaller way.

https://snag.gy/Vfem6d.jpg

I will do a mock up during the day to present you what I have in mind.
We need at least to remove the = sign which should be part of the config file.

On qml file, we will need to couple questions and answers to avoid mistakes while entering questions and answer in the future qml config file.

Good luck :)

@echarruau

Diff Update:

  1. Made the answer text slide down when question length is too long
  2. Moved the buttons up when window height > width

Updates to be made:

  1. Remove the submit button
  2. Add some sort of timer
  3. Create separate json files for different types of questions instead of one
src/activities/question_and_answer/resource/dataset.json
1 ↗(On Diff #51033)

Okay, I get it now, I'll get on it. Thanks :)

https://snag.gy/AvZwbC.jpg
@echarruau when the question length is very long, even if the window.width > window.height, the rearrangement will take place, that is why i said that it has nothing to do with the playarea's width, and for this rearrangement i made use of states, without using states this question would have looked like, https://snag.gy/MwyHUI.jpg

Ok, I will see with Johnny if state is the good tool for it, I don't remember saying it used for this purpose.
About your last comment I did not ask you to remove submit button but the start button.
I will prepare a mockup to explain you the size of the fonts I have in mind, the test you have done on sentences length could also be useful there implement the change I have in mind.

Here is the mockup I prepared for long sentences
https://snag.gy/IQLv5e.jpg

Here is the mockup I prepared for long sentences
https://snag.gy/IQLv5e.jpg

It's a mockup but wouldn't it make cleaner to have all the answer zones aligned?
On phones, it may be better to have the answer zone below the question too.

No issue at all with using states, let's use all qml capabilities :)

src/activities/question_and_answer/Question_and_answer.qml
69

it's really not a string?

src/activities/question_and_answer/question_and_answer.js
84

the name of the function is misleading. A getter is just to get a value, not to compute things and change current state of an object

Seeing Johnny's question I will try to be more precise about the mockup.
I would like to see the answer area close to the question, without equal. Take a resonnably large answer length. If the area fits on the same line keep it on the same line.
If it does not fit put it alone on the next line. The size of the answer area should be adaptive. It should increase with the number of characters entered if the answer is long.
This is because if from the beginning you fit the length of the answer area to the answer itself it will give hints for the pupil to choose the answer.

AkshayCHD updated this revision to Diff 52712.Feb 27 2019, 10:29 AM

Made the Following changes

  1. Removed the two standard buttons and added single OK button in there place
  2. Added image and animation to indicate that time is running
  3. Submission button is visible when the first answer text is clicked
  4. Splitting the json file into several smaller json files

Changes left to be Implemented
Making the answer field adaptive to the answer size

AkshayCHD updated this revision to Diff 55045.Mar 29 2019, 7:52 PM

Added the adaptive TextArea in place of TextField that adjusts its size according to the text entered

@echarruau Hi, i have updated the diff, please have a look whenever you have time :)

AkshayCHD updated this revision to Diff 55046.Mar 29 2019, 8:00 PM

Just a quick note, can you please fix those:
questions_answers.diff:740: trailing whitespace.

implicitHeight: Math.max(grid.cellHeight / 2, 40)

questions_answers.diff:981: trailing whitespace.

for( var i = 0; i < items.questions.length; i++){

questions_answers.diff:1166: trailing whitespace.

questions_answers.diff:1200: trailing whitespace.

questions_answers.diff:1234: trailing whitespace.

A few notes:

  • The selected question should have a visual feedback
  • There should be scroll buttons on the side to tell there are questions outside the visual zone
  • The tab button does not give focus in an intuitive order (not from left to right or top to bottom)
  • For text answers, the text area should contain multiple lines. Having only one line prevents knowing what has been written before.
  • Questions are outside panel range and answer area can override answer: https://pasteboard.co/I7XcMoO6.png
AkshayCHD updated this revision to Diff 56240.Apr 14 2019, 7:29 PM

Made the following changes

  • Added visual feedback for selected question(made the border color grey)
  • Added the scroll buttons on the side
  • Tab works intuitively
  • Made the input text span multiple lines for longer texts.
  • Fixed to some extent the problem of input texts being overridden even outside the panel, but some problem still exists for longer texts(working to solve that completely)

@jjazeix I have updated the diff, please have a look whenever you have time :)

Hi,
I am working from a fresh version of master and get the same problem when I want to apply diff for both D18230.diff
and D18600.diff.
error: patch failed: src/activities/activities.txt:119
Could you on your side apply your patch to see if it is working?
Thanks

Hi,
I am working from a fresh version of master and get the same problem when I want to apply diff for both D18230.diff

and D18600.diff.

error: patch failed: src/activities/activities.txt:119
Could you on your side apply your patch to see if it is working?
Thanks

That's normal. That's 2 separate independant patches doing different things. If they collide, there is nothing to do, just apply one or the other. Or update the second patch to not update activities.txt file and do the change by hand after applying the patch.

@AkshayCHD Are you still working on this activity? If not can you please let me know what is still left to be done I would like to take this up and complete it.
Thanks:)

@dekumar Hi, this activity is towards completion, I think most of the functionality has been implemented so I intent to complete it along with ascending_order activity.

@dekumar Hi, this activity is towards completion, I think most of the functionality has been implemented so I intent to complete it along with ascending_order activity.

Okay. Thanks:)