D18230: Added admin mode to question and answer activity

Johnny Jazeix noreply at phabricator.kde.org
Wed Jan 23 12:32:51 GMT 2019


jjazeix added inline comments.

INLINE COMMENTS

> ActivityInfo.qml:27
> +  //: Activity title
> +  title: "Question_and_answer activity"
> +  //: Help title

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

> Question_and_answer.qml:44
> +        Component.onCompleted: {
> +            activity.start.connect(start)
> +            activity.stop.connect(stop)

missing dialogActivityConfig.getInitialConfiguration() to initialize the config dialog

> Question_and_answer.qml:68
> +            property var allQuestions: []
> +            property var longestQuestion: ""
> +            property int questionsCount: 0

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

> Question_and_answer.qml:216
> +
> +        DialogActivityConfig {
> +            id: dialogActivityConfig

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.

> Question_and_answer.qml:234
> +                                items.currentMode = "freeMode"
> +                                adminMode.checked = false
> +

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

> Question_and_answer.qml:269
> +                        style: CheckBoxStyle {
> +                            label: Text {
> +                            text: items.longestQuestion

use GCText, not Text to have the same font everywhere

> dataset.js:25
> +                {
> +                    category: "Multiplication",
> +                    category_image_src: "qrc:/gcompris/src/activities/algebra_by/algebra_by.svg",

qsTr missing?

> question_and_answer.js:139
> +
> +function setLongestQuestion() {
> +    var long = ""

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

REPOSITORY
  R2 GCompris

REVISION DETAIL
  https://phabricator.kde.org/D18230

To: AkshayCHD, #gcompris_activities
Cc: jjazeix, echarruau, kde-edu, #gcompris_activities, harrymecwan, ganeshredcobra, nityanandkumar, rahulyadav, narvaez, scagarwal, apol, timotheegiet, bcoudoin
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20190123/2e684736/attachment.html>


More information about the kde-edu mailing list