Code review comment for lp:~yohanboniface/ubuntu-calendar-app/AgendaView

Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

> @Kunal: I suggest to keep code DRY and well organised/separated, and thus to
> keep the Date custom methods in DateExt (instead of putting generic functions
> in a qml file), and to update there when we know how to properly handle date
> localization in JavaScript. Thoughts?

I personally would prefer correctness over structure of code. Not to say that your code is not correct, but as I said
earlier, I was suggested to use following over Qt.formatDate, so I will suggest you to use the same.

        var timeFormat = i18n.tr("hh:mm");
        var startTime = e.startDateTime.toLocaleTimeString(Qt.locale(), timeFormat)

But if you must use function then I have following suggestion.
- Create a QtObject and move format function there, like done in Defines.js, getReminderLabels() function. that will allow usage of i18n.tr with toLocaleTimeString.

- now that we are using toLocaleTimeString, we can have only single format function that takes DATE/TIME format as argument and we can have constant with well defined name and use function with those constants.

« Back to merge proposal