After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 705050 - show local time ; “gnome-cities”
show local time ; “gnome-cities”
Status: RESOLVED FIXED
Product: gnome-weather
Classification: Applications
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: GNOME Weather Maintainer(s)
GNOME Weather Maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2013-07-28 22:57 UTC by Arnaud B.
Modified: 2014-04-01 21:07 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
This shows local-time for location in city-view (8.51 KB, patch)
2014-02-18 16:42 UTC, Saurabh_P
needs-work Details | Review
This shows local-time for location in city-view (5.95 KB, patch)
2014-02-19 15:58 UTC, Saurabh_P
committed Details | Review
This shows local-time for location in city-view (6.03 KB, patch)
2014-02-19 17:07 UTC, Saurabh_P
none Details | Review

Description Arnaud B. 2013-07-28 22:57:09 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 ).
Comment 1 Saurabh_P 2014-02-18 16:42:00 UTC
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.
Comment 2 Giovanni Campagna 2014-02-18 20:36:23 UTC
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)
Comment 3 Saurabh_P 2014-02-19 15:58:44 UTC
Created attachment 269697 [details] [review]
This shows local-time for location in city-view

Corrected patch.
Comment 4 Giovanni Campagna 2014-02-19 16:46:37 UTC
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.
Comment 5 Saurabh_P 2014-02-19 17:07:45 UTC
Created attachment 269707 [details] [review]
This shows local-time for location in city-view

corrected again.
Comment 6 Giovanni Campagna 2014-04-01 21:06:59 UTC
Pushed.