D18600: Improvements in ascending_order activity

[Aman Kumar Gupta]

amankumargupta added a comment.


  Working of the activity seems good to me. Will look into the code more in-depth in free time.

INLINE COMMENTS

> OrderingNumbers.qml:80
> +            fontSize: tinySize
> +            text: items.orderingType === "ascending" ? qsTr("Drag and drop the items in correct position in Ascending order") : qsTr("Drag and drop the items in correct position in Descending order")
> +            horizontalAlignment: Text.Center

Can you enclose items.orderingType === "ascending" within a bracket? Improves readibility.

> OrderingNumbers.qml:103
> +        Rectangle {
> +            id: container
> +            width: Math.min(parent.width, ((boxes.itemAt(0)).width*boxes.model)+(boxes.model-1)*flow.spacing)

Can you find a better name for it?

> OrderingNumbers.qml:104
> +            id: container
> +            width: Math.min(parent.width, ((boxes.itemAt(0)).width*boxes.model)+(boxes.model-1)*flow.spacing)
> +            height: parent.height/2

missing indenting spaces between operators and operands.

> OrderingNumbers.qml:114
> +            Flow {
> +                id: flow
> +                spacing: 10

Please find a better name for it.

> OrderingNumbers.qml:126
> +                anchors {
> +                    fill: parent
> +                }

if it's just one anchor property, why not just do anchors.fill: parent? :)

> OrderingNumbers.qml:135
> +                        color: "white"
> +                        z: mouseArea.drag.active ||  mouseArea.pressed ? 2 : 1
> +                        property string boxValue

Please enclose mouseArea.drag.active || mouseArea.pressed within a bracket to improve readibility

> OrderingNumbers.qml:168
> +                            anchors.margins: ApplicationInfo.ratio * 5
> +                            source: items.tileType == "image" ? boxValue : ""
> +                        }

same comment regarding enclosing within bracket

> OrderingNumbers.qml:282
> +          sourceSize.width: 70 * ApplicationInfo.ratio
> +            anchors {
> +              right: parent.right

Indentation.

REPOSITORY
  R2 GCompris

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

To: AkshayCHD, #gcompris_activities, jjazeix
Cc: amankumargupta, jjazeix, echarruau, kde-edu, #gcompris_activities, sanjayshetty, parimalprasoon, harrymecwan, ganeshredcobra, nityanandkumar, andreask, rahulyadav, narvaez, scagarwal, apol, timotheegiet, bcoudoin
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20190418/094293cb/attachment-0001.html>