GNOME Bugzilla – Bug 705050
show local time ; “gnome-cities”
Last modified: 2014-04-01 21:07:13 UTC
Displaying the current temperature of a city without the local hour looks strange, the temperature really depends of the hour of the day… And of course, it would be better to show at the same time the hours of begin and end of the day (and the solar hour ?), but it’s more difficult. More generally, if you’re interested in a city, it’s not only for weather: it’s because there is people you know, it’s your next destination… you may want to know a little more than just weather (have link to a Maps app’, for example). Thinking of “gnome-cities” (“Locations” ?) would be more interesting. “Clocks” might at this moment become “Alarms”, for time-management (and so lost bugs like https://bugzilla.gnome.org/show_bug.cgi?id=690376 ).
Created attachment 269576 [details] [review] This shows local-time for location in city-view I've modified city.ui file using glade 3.16.1 so the corrected code in city.ui is generated by it. This patch is helping in showing local-time for given location in city-view in the top-left corner and I've discussed this design with AllanDay.
Review of attachment 269576 [details] [review]: There are some unrelated changes to city.ui that would be nice to avoid. git checkout -p / git add -p can help you there. ::: data/city.ui @@ -34,3 @@ - <property name="accessible-name" translatable="yes">Current conditions</property> - </object> - </child> Don't lose the accessible! ::: src/city.js @@ +128,3 @@ this.get_accessible().accessible_name = _("City view"); + this._wallClock = new Gnome.WallClock(); + this._wallClock.connect('notify::clock', Lang.bind(this, this._updateTime)); You need to disconnect this signal when exiting the city view, otherwise we keep ticking in the background for no reason. @@ +203,3 @@ + let location = this._info.location; + let tz = GLib.TimeZone.new(location.get_timezone().get_tzid()); + let dt = new GLib.DateTime(); I'm surprised this constructor syntax works, but in general, with boxed types with more than one constructor, you need to use the call form. So, GLib.DateTime.new_now(timezone); (And you avoid the to_timezone() call below)
Created attachment 269697 [details] [review] This shows local-time for location in city-view Corrected patch.
Review of attachment 269697 [details] [review]: Looks almost good. But, because 3.11.90 is already out, this requires a UI freeze break from the release-team. I don't think it will be a problem to obtain one, we're early in the release cycle. Btw, it's a lot easier to obtain a freeze break if you include a screenshot of the app with the new feature. ::: src/city.js @@ +193,3 @@ + }, + + _connectClock: function() { Make this and _disconnectClock public (ie, avoid the underscore) @@ +207,3 @@ + let tz = GLib.TimeZone.new(location.get_timezone().get_tzid()); + let dt = GLib.DateTime.new_now(tz); + return dt.format("%H:%M"); This string needs to be localized, and you should add a comment for translators, to explain what it is used for.
Created attachment 269707 [details] [review] This shows local-time for location in city-view corrected again.
Pushed.