D17783: Add configuration menu in small numbers activity to support multiple domino types

Johnny Jazeix noreply at phabricator.kde.org
Wed Jan 2 10:20:23 GMT 2019


jjazeix added inline comments.

INLINE COMMENTS

> Gletters.qml:79
>          Component.onCompleted: {
> -            dialogActivityConfig.getInitialConfiguration()
> +            if (activity.activityName == "gletters")
> +                dialogActivityConfig.getInitialConfiguration()

I would prefer to only have one dialogActivityConfig and play with visible property instead

> Smallnumbers2.qml:35
> +
> +    QtObject {
> +            id: items

just having a property string dominoMode would be simplier?

> Smallnumbers2.qml:47
> +                property var availableModes: [
> +                    { "text": qsTr("Dots"), "value": "dot" },
> +                    { "text": qsTr("Numbers"), "value": "number" },

As it is reused at multiple places, I think it would be better to have this array in the Domino itself.
This will avoid to miss one place if we add a new mode.

REPOSITORY
  R2 GCompris

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

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


More information about the kde-edu mailing list