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 724416 - 'Add' button is sensitive for unavailable location in new location dialog box
'Add' button is sensitive for unavailable location in new location dialog box
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)
: 724459 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2014-02-15 12:03 UTC by Saurabh_P
Modified: 2014-02-17 21:36 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
this changes sensitivity of "Add" button in new location dialog box whenever it needs to be changed. (2.02 KB, patch)
2014-02-15 16:31 UTC, Saurabh_P
reviewed Details | Review
Corrected patch (1.79 KB, patch)
2014-02-17 13:52 UTC, Saurabh_P
none Details | Review
Corrected patch (1.79 KB, patch)
2014-02-17 13:57 UTC, Saurabh_P
committed Details | Review

Description Saurabh_P 2014-02-15 12:03:55 UTC
When user enters invalid or unavailable location in GtkLocationEntry, the "Add" button remains sensitive and if user presses add then the dialog box gets destroyed and affects nothing. Instead of this User should not be able to press "Add" button when entered location is not available or nothing is entered.
Comment 1 Saurabh_P 2014-02-15 12:19:37 UTC
If I press return without entering any location in the new location dialog box, t shows segfault and crashes down.
Comment 2 Saurabh_P 2014-02-15 16:31:22 UTC
Created attachment 269216 [details] [review]
this changes sensitivity of "Add" button in  new location dialog box whenever it needs to be changed.

This patch might have an issue with the look of ADD button when it is not sensitive but that has to do with gnome-themes-standard(dark theme) and not gnome-weather.
Comment 3 Giovanni Campagna 2014-02-15 16:54:49 UTC
Review of attachment 269216 [details] [review]:

::: src/window.js
@@ +54,2 @@
         dialog.connect('response', Lang.bind(this, this._onResponse));
+        entry.connect('search-changed', Lang.bind(this, this._location_changed));

search-changed only happens after a timeout. You should use notify::location instead.

@@ +77,3 @@
+    },
+
+    _location_changed: function() {

Please use lowerCamelCase, following the gjs convention.

@@ +80,3 @@
+        var empty = this._entry.get_text().localeCompare("") == 0;
+
+        if (!this._entry.location || empty) {

This empty check is not needed anymore, the libgweather crash was fixed.

@@ +81,3 @@
+
+        if (!this._entry.location || empty) {
+            this._dialog.set_default_response(Gtk.ResponseType.CANCEL);

Mh, so if I make a typo searching and press enter it cancels? This does not look like an improvement from before...
I'd keep the default response to OK, and make it insensitive when location == null.
Comment 4 Saurabh_P 2014-02-15 19:18:35 UTC
Let's not change default response as it is not making much sense but we have to check for empty location entry because if user enters some location and then erase it completely then also the add button remains sensitive as user entered one valid location before so we have to manually set location to null whenever location entry is emptied.
Comment 5 Saurabh_P 2014-02-15 19:40:27 UTC
Connect "notify::location" signal causes some problem to locationentry dialog-box. Clicking on new button tries to open the new location dialog box but the box is blacked out and after that I've to do force-quit. Connecting this signal from glade doesn't work and I already have this experience.
Comment 6 Saurabh_P 2014-02-16 09:24:25 UTC
Delay for 'search-change' emission after last change is only 150 milliseconds. I guess this would not make any major difference.
Comment 7 Saurabh_P 2014-02-17 13:52:30 UTC
Created attachment 269396 [details] [review]
Corrected patch

Using "notify::location" messes up with new location entry dialog box(make it totally black) and I've to force quit the app. We have to check for empty condition as to set the entry.location to null if user entered valid location before the empty condition otherwise it'll show that the entry.location is set to the previously entered location.
Comment 8 Saurabh_P 2014-02-17 13:57:01 UTC
Created attachment 269398 [details] [review]
Corrected patch

Using "notify::location" messes up with new location entry dialog box(make it totally black) and I've to force quit the app. We have to check for empty condition as to set the entry.location to null if user entered valid location before the empty condition otherwise it'll show that the entry.location is set to the previously entered location.
Comment 9 Giovanni Campagna 2014-02-17 21:36:07 UTC
Pushed with the notify::location change, after testing that it works.
Comment 10 Giovanni Campagna 2014-02-17 21:36:28 UTC
*** Bug 724459 has been marked as a duplicate of this bug. ***