Re: Review Request 120358: DigitalClock: Add timezone information to digital clock tooltip.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120358/ --- (Updated Sept. 29, 2014, 4:34 p.m.) Status -- This change has been marked as submitted. Review request for Plasma, Kai Uwe Broulik and Martin Klapetek. Repository: plasma-workspace Description --- DigitalClock: Add timezone information to digital clock tooltip. note: My editor is dumb and removed the umlaut from Sebas' name, I'll fix that before committing. Diffs - applets/digital-clock/package/contents/ui/DigitalClock.qml 2beae00d090dfe03e94c4223ba36546ffeda45f2 Diff: https://git.reviewboard.kde.org/r/120358/diff/ Testing --- I ran it here with two timezones selected (Local and London). The date is local date, but it was that way before anyway. The date next to each timezone is the date for that timezone if dates are shown. Thanks, Jeremy Whiting ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120358: DigitalClock: Add timezone information to digital clock tooltip.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120358/ --- (Updated Sept. 27, 2014, 1:51 p.m.) Review request for Plasma, Kai Uwe Broulik and Martin Klapetek. Changes --- Added another newline between date and timezone strings. Repository: plasma-workspace Description --- DigitalClock: Add timezone information to digital clock tooltip. note: My editor is dumb and removed the umlaut from Sebas' name, I'll fix that before committing. Diffs (updated) - applets/digital-clock/package/contents/ui/DigitalClock.qml 2beae00d090dfe03e94c4223ba36546ffeda45f2 Diff: https://git.reviewboard.kde.org/r/120358/diff/ Testing --- I ran it here with two timezones selected (Local and London). The date is local date, but it was that way before anyway. The date next to each timezone is the date for that timezone if dates are shown. Thanks, Jeremy Whiting ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120358: DigitalClock: Add timezone information to digital clock tooltip.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120358/#review67538 --- applets/digital-clock/package/contents/ui/DigitalClock.qml https://git.reviewboard.kde.org/r/120358/#comment47079 The result of both of these i18ncs is exactly the same; plus both comments are not true if it's used for the tooltip. However, changing the comments would violate string freeze. I'd suggest to possibly remove the i18nc altogether here as there are no localizable things (besides the order, but I guess we could live with that for a while). - Martin Klapetek On Sept. 27, 2014, 9:51 p.m., Jeremy Whiting wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120358/ --- (Updated Sept. 27, 2014, 9:51 p.m.) Review request for Plasma, Kai Uwe Broulik and Martin Klapetek. Repository: plasma-workspace Description --- DigitalClock: Add timezone information to digital clock tooltip. note: My editor is dumb and removed the umlaut from Sebas' name, I'll fix that before committing. Diffs - applets/digital-clock/package/contents/ui/DigitalClock.qml 2beae00d090dfe03e94c4223ba36546ffeda45f2 Diff: https://git.reviewboard.kde.org/r/120358/diff/ Testing --- I ran it here with two timezones selected (Local and London). The date is local date, but it was that way before anyway. The date next to each timezone is the date for that timezone if dates are shown. Thanks, Jeremy Whiting ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120358: DigitalClock: Add timezone information to digital clock tooltip.
On Sept. 27, 2014, 2:05 p.m., Martin Klapetek wrote: applets/digital-clock/package/contents/ui/DigitalClock.qml, lines 257-263 https://git.reviewboard.kde.org/r/120358/diff/3/?file=315594#file315594line257 The result of both of these i18ncs is exactly the same; plus both comments are not true if it's used for the tooltip. However, changing the comments would violate string freeze. I'd suggest to possibly remove the i18nc altogether here as there are no localizable things (besides the order, but I guess we could live with that for a while). I'm not sure what you are suggesting here? Remove the i18nc from the new string altogether? - Jeremy --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120358/#review67538 --- On Sept. 27, 2014, 1:51 p.m., Jeremy Whiting wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120358/ --- (Updated Sept. 27, 2014, 1:51 p.m.) Review request for Plasma, Kai Uwe Broulik and Martin Klapetek. Repository: plasma-workspace Description --- DigitalClock: Add timezone information to digital clock tooltip. note: My editor is dumb and removed the umlaut from Sebas' name, I'll fix that before committing. Diffs - applets/digital-clock/package/contents/ui/DigitalClock.qml 2beae00d090dfe03e94c4223ba36546ffeda45f2 Diff: https://git.reviewboard.kde.org/r/120358/diff/ Testing --- I ran it here with two timezones selected (Local and London). The date is local date, but it was that way before anyway. The date next to each timezone is the date for that timezone if dates are shown. Thanks, Jeremy Whiting ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120358: DigitalClock: Add timezone information to digital clock tooltip.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120358/ --- (Updated Sept. 27, 2014, 2:38 p.m.) Review request for Plasma, Kai Uwe Broulik and Martin Klapetek. Changes --- Moved tooltip code into a separate method to always call when datasource changes. Also call it initially to get the tooltip right at first launch. Repository: plasma-workspace Description --- DigitalClock: Add timezone information to digital clock tooltip. note: My editor is dumb and removed the umlaut from Sebas' name, I'll fix that before committing. Diffs (updated) - applets/digital-clock/package/contents/ui/DigitalClock.qml 2beae00d090dfe03e94c4223ba36546ffeda45f2 Diff: https://git.reviewboard.kde.org/r/120358/diff/ Testing --- I ran it here with two timezones selected (Local and London). The date is local date, but it was that way before anyway. The date next to each timezone is the date for that timezone if dates are shown. Thanks, Jeremy Whiting ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120358: DigitalClock: Add timezone information to digital clock tooltip.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120358/ --- (Updated Sept. 27, 2014, 2:53 p.m.) Review request for Plasma, Kai Uwe Broulik and Martin Klapetek. Changes --- Removed unneeded extra i18nc call that was the same as another one. Updated context string for translators. Repository: plasma-workspace Description --- DigitalClock: Add timezone information to digital clock tooltip. note: My editor is dumb and removed the umlaut from Sebas' name, I'll fix that before committing. Diffs (updated) - applets/digital-clock/package/contents/ui/DigitalClock.qml 2beae00d090dfe03e94c4223ba36546ffeda45f2 Diff: https://git.reviewboard.kde.org/r/120358/diff/ Testing --- I ran it here with two timezones selected (Local and London). The date is local date, but it was that way before anyway. The date next to each timezone is the date for that timezone if dates are shown. Thanks, Jeremy Whiting ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120358: DigitalClock: Add timezone information to digital clock tooltip.
On Sept. 27, 2014, 2:05 p.m., Martin Klapetek wrote: applets/digital-clock/package/contents/ui/DigitalClock.qml, lines 257-263 https://git.reviewboard.kde.org/r/120358/diff/3/?file=315594#file315594line257 The result of both of these i18ncs is exactly the same; plus both comments are not true if it's used for the tooltip. However, changing the comments would violate string freeze. I'd suggest to possibly remove the i18nc altogether here as there are no localizable things (besides the order, but I guess we could live with that for a while). Jeremy Whiting wrote: I'm not sure what you are suggesting here? Remove the i18nc from the new string altogether? String freeze exception requested here: http://lists.kde.org/?l=kde-i18n-docm=141185139919104w=2 - Jeremy --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120358/#review67538 --- On Sept. 27, 2014, 2:53 p.m., Jeremy Whiting wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120358/ --- (Updated Sept. 27, 2014, 2:53 p.m.) Review request for Plasma, Kai Uwe Broulik and Martin Klapetek. Repository: plasma-workspace Description --- DigitalClock: Add timezone information to digital clock tooltip. note: My editor is dumb and removed the umlaut from Sebas' name, I'll fix that before committing. Diffs - applets/digital-clock/package/contents/ui/DigitalClock.qml 2beae00d090dfe03e94c4223ba36546ffeda45f2 Diff: https://git.reviewboard.kde.org/r/120358/diff/ Testing --- I ran it here with two timezones selected (Local and London). The date is local date, but it was that way before anyway. The date next to each timezone is the date for that timezone if dates are shown. Thanks, Jeremy Whiting ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120358: DigitalClock: Add timezone information to digital clock tooltip.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120358/#review67541 --- Ship it! If the translators won't object, ship it from me while closing bug 337564. And thanks :) applets/digital-clock/package/contents/ui/DigitalClock.qml https://git.reviewboard.kde.org/r/120358/#comment47083 ...current time in that zone applets/digital-clock/package/contents/ui/DigitalClock.qml https://git.reviewboard.kde.org/r/120358/#comment47082 Remove this empty space - Martin Klapetek On Sept. 27, 2014, 10:53 p.m., Jeremy Whiting wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120358/ --- (Updated Sept. 27, 2014, 10:53 p.m.) Review request for Plasma, Kai Uwe Broulik and Martin Klapetek. Repository: plasma-workspace Description --- DigitalClock: Add timezone information to digital clock tooltip. note: My editor is dumb and removed the umlaut from Sebas' name, I'll fix that before committing. Diffs - applets/digital-clock/package/contents/ui/DigitalClock.qml 2beae00d090dfe03e94c4223ba36546ffeda45f2 Diff: https://git.reviewboard.kde.org/r/120358/diff/ Testing --- I ran it here with two timezones selected (Local and London). The date is local date, but it was that way before anyway. The date next to each timezone is the date for that timezone if dates are shown. Thanks, Jeremy Whiting ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120358: DigitalClock: Add timezone information to digital clock tooltip.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120358/#review67406 --- Can you provide a screenshot of the tooltip? applets/digital-clock/package/contents/ui/DigitalClock.qml https://git.reviewboard.kde.org/r/120358/#comment47049 Simple bindings can be written without braces and return: text: timeForZone(tzIndex, true) applets/digital-clock/package/contents/ui/DigitalClock.qml https://git.reviewboard.kde.org/r/120358/#comment47050 ++i applets/digital-clock/package/contents/ui/DigitalClock.qml https://git.reviewboard.kde.org/r/120358/#comment47048 This looks wrong to me. You are using the same format that is used in the clock itself for the tooltip? In 4.x the tooltip is: Thursday, 25 September 2014 Berlin: 11:31 London: 10:31 Toronto: 4:31 - Kai Uwe Broulik On Sept. 25, 2014, 2:12 vorm., Jeremy Whiting wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120358/ --- (Updated Sept. 25, 2014, 2:12 vorm.) Review request for Plasma, Kai Uwe Broulik and Martin Klapetek. Repository: plasma-workspace Description --- DigitalClock: Add timezone information to digital clock tooltip. note: My editor is dumb and removed the umlaut from Sebas' name, I'll fix that before committing. Diffs - applets/digital-clock/package/contents/ui/DigitalClock.qml 00c9def8dffe1cdc24d377acab029b47bd57e602 Diff: https://git.reviewboard.kde.org/r/120358/diff/ Testing --- I ran it here with two timezones selected (Local and London). The date is local date, but it was that way before anyway. The date next to each timezone is the date for that timezone if dates are shown. Thanks, Jeremy Whiting ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120358: DigitalClock: Add timezone information to digital clock tooltip.
On Sept. 25, 2014, 3:33 a.m., Kai Uwe Broulik wrote: Can you provide a screenshot of the tooltip? I'm using the same function to create the time format. but with a different argument. I guess I could make two separate functions instead, but why not just use the same one, when 90% of the code is the same? Anyway, I've added a couple screenshots http://imgur.com/biI6prN,lbxJS4k and http://imgur.com/biI6prN,lbxJS4k#1 though when I look at it admittedly the withDate one looks funny, and should have the timezone first like the other one and outside of () also. I'll fix that and post an update to this review. - Jeremy --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120358/#review67406 --- On Sept. 24, 2014, 8:12 p.m., Jeremy Whiting wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120358/ --- (Updated Sept. 24, 2014, 8:12 p.m.) Review request for Plasma, Kai Uwe Broulik and Martin Klapetek. Repository: plasma-workspace Description --- DigitalClock: Add timezone information to digital clock tooltip. note: My editor is dumb and removed the umlaut from Sebas' name, I'll fix that before committing. Diffs - applets/digital-clock/package/contents/ui/DigitalClock.qml 00c9def8dffe1cdc24d377acab029b47bd57e602 Diff: https://git.reviewboard.kde.org/r/120358/diff/ Testing --- I ran it here with two timezones selected (Local and London). The date is local date, but it was that way before anyway. The date next to each timezone is the date for that timezone if dates are shown. Thanks, Jeremy Whiting ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120358: DigitalClock: Add timezone information to digital clock tooltip.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120358/ --- (Updated Sept. 25, 2014, 11:49 a.m.) Review request for Plasma, Kai Uwe Broulik and Martin Klapetek. Changes --- Updated timeForZone to be a bit cleaner in separating the two cases of showing line breaks or not (main ui, vs tooltip) Fixed other minor issues reported on reviewboard also. Repository: plasma-workspace Description --- DigitalClock: Add timezone information to digital clock tooltip. note: My editor is dumb and removed the umlaut from Sebas' name, I'll fix that before committing. Diffs (updated) - applets/digital-clock/package/contents/ui/DigitalClock.qml 00c9def8dffe1cdc24d377acab029b47bd57e602 Diff: https://git.reviewboard.kde.org/r/120358/diff/ Testing --- I ran it here with two timezones selected (Local and London). The date is local date, but it was that way before anyway. The date next to each timezone is the date for that timezone if dates are shown. Thanks, Jeremy Whiting ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120358: DigitalClock: Add timezone information to digital clock tooltip.
On Sept. 25, 2014, 9:33 vorm., Kai Uwe Broulik wrote: Can you provide a screenshot of the tooltip? Jeremy Whiting wrote: I'm using the same function to create the time format. but with a different argument. I guess I could make two separate functions instead, but why not just use the same one, when 90% of the code is the same? Anyway, I've added a couple screenshots http://imgur.com/biI6prN,lbxJS4k and http://imgur.com/biI6prN,lbxJS4k#1 though when I look at it admittedly the withDate one looks funny, and should have the timezone first like the other one and outside of () also. I'll fix that and post an update to this review. Fair enough. Looks better than I expected. Yes, the date and the order looks awkward, and please add an empty line between the current date and the list of time zones. Thanks! - Kai Uwe --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120358/#review67406 --- On Sept. 25, 2014, 5:49 nachm., Jeremy Whiting wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120358/ --- (Updated Sept. 25, 2014, 5:49 nachm.) Review request for Plasma, Kai Uwe Broulik and Martin Klapetek. Repository: plasma-workspace Description --- DigitalClock: Add timezone information to digital clock tooltip. note: My editor is dumb and removed the umlaut from Sebas' name, I'll fix that before committing. Diffs - applets/digital-clock/package/contents/ui/DigitalClock.qml 00c9def8dffe1cdc24d377acab029b47bd57e602 Diff: https://git.reviewboard.kde.org/r/120358/diff/ Testing --- I ran it here with two timezones selected (Local and London). The date is local date, but it was that way before anyway. The date next to each timezone is the date for that timezone if dates are shown. Thanks, Jeremy Whiting ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120358: DigitalClock: Add timezone information to digital clock tooltip.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120358/ --- (Updated Sept. 24, 2014, 8:12 p.m.) Review request for Plasma, Kai Uwe Broulik and Martin Klapetek. Repository: plasma-workspace Description --- DigitalClock: Add timezone information to digital clock tooltip. note: My editor is dumb and removed the umlaut from Sebas' name, I'll fix that before committing. Diffs - applets/digital-clock/package/contents/ui/DigitalClock.qml 00c9def8dffe1cdc24d377acab029b47bd57e602 Diff: https://git.reviewboard.kde.org/r/120358/diff/ Testing --- I ran it here with two timezones selected (Local and London). The date is local date, but it was that way before anyway. The date next to each timezone is the date for that timezone if dates are shown. Thanks, Jeremy Whiting ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
