GNOME Bugzilla – Bug 764596
Better feedback when geocode lookup fails
Last modified: 2017-06-20 19:47:03 UTC
Looking for: 574 chemin de wette fays 69300 caluire in the search entry, or in any of the 2 directions entry doesn't give any feedback. The search results entry will pop down, and then disappear without feedback. You could use the search results popover to show that no results were found and hints on how to fix it, for the main search, and use the "error" CSS class on the directions entries when lookup fails. For the record, removing the postcode (69300) makes the search work, but that's a work-around.
Anyone help with this.
(In reply to Neha yadav from comment #1) > Anyone help with this. Hi! You need to be more specific than that! What kind of help are you looking for? You can try the #newcomers irc channel for general gnome development help or #gnome-maps for specific Maps help You can look through the Maps wiki: https://wiki.gnome.org/Apps/Maps/
Some mockups would be nice for this. Both for the route panel and for the search popdown I guess. Andreas: Would you mind looking into that when/if you get time?
Yes, I'm happy to take a look.
We have to remove line 217 from this file https://git.gnome.org/browse/gnome-maps/tree/src/placeEntry.js#n217. In place of this I am creating a showNoResult() inside place popover, which in response show "No result found". Now the problem is that I am not understanding some part of code like how result is showing when we press enter key.
Created attachment 335771 [details] [review] search-result: show no result found in popover The problem is that search entry doesn't give any feedback if result were not found. The search results entry will pop down, and then disappear without feedback. To fix this create a method showNoResult() in placePopover.js and call that in resonse to the "No result found". Remove line 217 from placeEntry.js and use showNoResult().
Review of attachment 335771 [details] [review]: catch! First some nits in the code (they are stated below). Then in the commit message: - keep your lines to max 72 chars - you don't need to be so specific about your changes (mentioning line numbers etc :)), it's enough to state the problem (like you did in the first line) and then that the fix is to add a "no results label". When those are fixed I found an issue that I'd want fixed before pushing this: When I do a search that contains results I can press ESC to hide the results popover, however I can't seem to do it when I don't get any results. Try to make that behaviour work as well! :) Great work, thanks for the patch! ::: data/ui/place-popover.ui @@ +67,3 @@ </child> + <child> + <object class="GtkLabel" id="fixedLabel"> Name this "noResultsLabel" @@ +70,3 @@ + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="label" translatable="yes">No result Found</property> "No result Found" ⇒ "No results found" ::: src/placePopover.js @@ +48,3 @@ 'spinner', + 'list', + 'fixedLabel' ], And then 'noResultsLabel' here. @@ +130,3 @@ + this._spinner.stop(); + + this._stack.visible_child = this._fixedLabel; …and here as well.
(In reply to Mattias Bengtsson from comment #7) > When I do a search that contains results I can press ESC to hide the > results popover, however I can't seem to do it when I don't get any > results. I did all the changes but I am not able to figure out how to hide the result popover. Can you help me with this.
(In reply to Neha yadav from comment #8) > (In reply to Mattias Bengtsson from comment #7) > > > When I do a search that contains results I can press ESC to hide the > > results popover, however I can't seem to do it when I don't get any > > results. > > I did all the changes but I am not able to figure out how to hide the result > popover. Can you help me with this. Not sure exactly, but I believe the answer lies in SearchPopover._propagateKeys. There's some handling for the Escape key right there. Not sure why it doesn't apply here though.
(In reply to Mattias Bengtsson from comment #9) > (In reply to Neha yadav from comment #8) > > (In reply to Mattias Bengtsson from comment #7) > > > > > When I do a search that contains results I can press ESC to hide the > > > results popover, however I can't seem to do it when I don't get any > > > results. > > > > I did all the changes but I am not able to figure out how to hide the result > > popover. Can you help me with this. > > Not sure exactly, but I believe the answer lies in > SearchPopover._propagateKeys. There's some handling for the Escape key right > there. Not sure why it doesn't apply here though. In searchPopover.js:_propagateKeys() there is some logic that finds the selected row (at the start of that function), I believe this leaves "row" unset in the no result case, as there's no selected row in that case (if I understand this correctly, get_row_at_index(0) will be undefined where's no list populated), so that the early return is triggered before checking the keyvals furher down.
Created attachment 353997 [details] [review] Patch including hide on escape I hope I'm not stepping on Neha Yadav's toes here but there hasn't been any activity on this bug in the last few months. So, I updated the existing patch to also hide the popover after clicking ESC.
(In reply to elias from comment #11) > Created attachment 353997 [details] [review] [review] > Patch including hide on escape > > I hope I'm not stepping on Neha Yadav's toes here but there hasn't been any > activity on this bug in the last few months. > > So, I updated the existing patch to also hide the popover after clicking ESC. Looks good for me from a UI point of view.
(In reply to elias from comment #11) > Created attachment 353997 [details] [review] [review] > Patch including hide on escape > > I hope I'm not stepping on Neha Yadav's toes here but there hasn't been any > activity on this bug in the last few months. > There's not been much activity on this one for a while, so no worries I'd say! > So, I updated the existing patch to also hide the popover after clicking ESC. Thanks for taking the time to look into this!
Review of attachment 353997 [details] [review]: I think the patch looks good! A little nitpick about the commit message: The sentence follwing the module name (after the colon) usually starts with a capital letter. Also, I think there could be some commit message body describing how it changes behaviour, i.e. something like: Instead of dismissing the result popover when no result where found, indicate there was no results found. A description of the commit message style can be found at: https://wiki.gnome.org/Git/CommitMessages
Created attachment 354077 [details] [review] search result: Show No result found in popover (commit message changed) I added a description to the the commit message.
Review of attachment 354077 [details] [review]: LGTM
Thanks!