D13574: Overall diff for GSoC tasks so far

Johnny Jazeix noreply at phabricator.kde.org
Sun Jun 17 08:03:12 UTC 2018


jjazeix added inline comments.

INLINE COMMENTS

> ActivityInfo.qml:3
> + *
> + * Copyright (C) 2016 Toncu Stefan <stefan.toncu29 at gmail.com>
> + *

add your name here

> ActivityInfo.qml:26
> +  demo: true
> +  title: "Drawing"
> +  description: ""

you need to complete the whole file here

> ColorDialogue.qml:1
> +/* GCompris - ColorDialogue.qml
> + *

should be named ColorDialog.qml

> ColorDialogue.qml:44
> +
> +    function updateColor(clr) {
> +        // QML does not expose any way of getting the components of a color

color, you can use the full name for the variable instead of abbrevations

> Drawing.qml:112
> +
> +        SaveToFilePrompt {
> +            id: saveToFilePrompt2

you can probably find a way to have only one SaveToFilePrompt used

> Drawing.qml:155
> +            property color paintColor: "#00ff00"
> +            property var urlImage
> +            property bool next: false

string?

> Drawing.qml:392
> +                    property var currentShape: items.toolSelected == "circle" ? circle : rectangle
> +                    property var originalX
> +                    property var originalY

real?

> Drawing.qml:877
> +                    enabled: items.toolSelected == "rectangle" || items.toolSelected == "line"|| items.toolSelected == "lineShift"
> +                    opacity: items.toolSelected == "rectangle" || items.toolSelected == "line"|| items.toolSelected == "lineShift" ? 1 : 0
> +

"enabled ? 1 : 0"?

> Drawing.qml:894
> +                    enabled: items.toolSelected == "circle"
> +                    opacity: items.toolSelected == "circle" ? 1 : 0
> +                    property real rotationn: 0

"enabled ? 1 : 0"?

> FoldablePanels.qml:107
> +        GCText {
> +            text: "Menu"
> +            fontSize: tinySize

qsTr()

> FoldablePanels.qml:117
> +
> +    Rectangle {
> +        id: toolsTitle

can you factorise it on a separate file? All the tabs should have more or less the same behaviour?

> FoldablePanels.qml:148
> +        GCText {
> +            text: "Tools"
> +            fontSize: tinySize

qsTr()

> FoldablePanels.qml:413
> +
> +        Button { style: GCButtonStyle { theme: "light" }
> +            text: qsTr("Save")

go to the line

> LoadDrawings.qml:29
> +Rectangle {
> +            id: load
> +            color: background.color

indentation

> LoadDrawings.qml:44
> +                anchors.fill: parent
> +                cellWidth: (background.width - exitButton.width) / 2 * slider1.value; cellHeight: cellWidth
> +                model: Activity.loadImagesSource

go to the line

> TextInputTool.qml:1
> +/* GCompris - TextInput.qml
> + *

TextInputTool.qml

> TextInputTool.qml:24
> +import QtQuick.Controls.Styles 1.4
> +import QtQuick.Dialogs 1.0
> +import "../../core"

not sure you need all the import on all files

> drawing.js:45
> +
> +var aux = ["1","2","3","4","5"]
> +

not a good variable name

REPOSITORY
  R2 GCompris

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

To: asagtani, #gcompris_activities
Cc: timotheegiet, jjazeix, kde-edu, asagtani, himanshuvishwakarma, harrymecwan, ganeshredcobra, nityanandkumar, echarruau, rahulyadav, narvaez, scagarwal, apol, hkaelberer, bcoudoin
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20180617/a63d3c97/attachment-0001.html>


More information about the kde-edu mailing list