D27903: [applet/digital-clock] Show time zones in expanded representation too

Carson Black noreply at phabricator.kde.org
Thu Mar 12 19:39:08 GMT 2020


cblack added inline comments.

INLINE COMMENTS

> CalendarView.qml:432
> +                        id: listItem
> +                        readonly property bool lastTimeZone: modelData === plasmoid.configuration.lastSelectedTimezone
>  

This property's name doesn't imply it's a boolean

> main.qml:62
> +        // get current UTC time
> +        var msUTC = now.getTime() + (now.getTimezoneOffset() * 60000);
> +        // add the dataengine TZ offset to it

This variable is only used in the following statement, so I would move it into there. Would probably make it multiple lines to not hurt readability.

> main.qml:69
> +        if (dateTime.getDay() !== dataSource.data["Local"]["DateTime"].getDay()) {
> +            formattedTime += " (" + Qt.formatDate(dateTime, compactRepresentationItem.dateFormat) + ")";
> +        }

Style change: use arg() instead of string concatenation so it's easier to see what the end string will look like

> main.qml:80
> +
> +        return timezoneString;
> +    }

Declaring a variable and then returning it immediately seems redundant.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  digital-clock-layouts-and-timezones-in-popup (branched from master)

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

To: ngraham, #vdg, #plasma, cblack
Cc: cblack, apol, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, ahiemstra, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20200312/2b665e6f/attachment.html>


More information about the Plasma-devel mailing list