GNOME Bugzilla – Bug 761634
Add support for editing website on OSM objects
Last modified: 2016-02-09 22:01:55 UTC
Since website is one of the additional fields of contact information we plan on showing with the new place popover design (which can show an expanded area with extra details) it makes sense to allow editing this field for OSM objects.
Created attachment 320536 [details] [review] osmEdit: Add support for editing website
Review of attachment 320536 [details] [review]: Thanks! Would it not be possible to add website to the bubble with the same commit? "Add website information to map bubbles"
(In reply to Jonas Danielsson from comment #2) > Review of attachment 320536 [details] [review] [review]: > > Thanks! > > Would it not be possible to add website to the bubble with the same commit? > > "Add website information to map bubbles" Absolutely, I'll add it. I guess we should add it as a clickable link like for Wikipedia. I'd like to make it show the actual URL, though, so that users could potentially see if the URL looks bogus. One concern might be overly long URLs though.
(In reply to Marcus Lundblad from comment #3) > (In reply to Jonas Danielsson from comment #2) > > Review of attachment 320536 [details] [review] [review] [review]: > > > > Thanks! > > > > Would it not be possible to add website to the bubble with the same commit? > > > > "Add website information to map bubbles" > > Absolutely, I'll add it. > I guess we should add it as a clickable link like for Wikipedia. > I'd like to make it show the actual URL, though, so that users could > potentially see if the URL looks bogus. One concern might be overly long > URLs though. Myabe as tooltip?
Created attachment 320661 [details] [review] osmEdit: Add support for editing website Also show a website link in the expanded place area when available. And fix a bug where the expanded area wasn't cleared properly when successfully updating an OSM object, when we're at it.
Attached a new patch that also adds the website link in the expanded area of the place bubble. For some reason, the tooltip isn't actually working, although it should be sufficent to set the "tooltip-text" property (I also added the "has-tooltip" property manually, still doesn't work). Manually changing "has-tooltip" to True using the Gtk Inspector works, so I thought I might as well attach the updated patch anyway, and we'll hopefully straighten things out…
Review of attachment 320661 [details] [review]: Thanks! ::: src/place.js @@ +282,3 @@ } else return null; +}; What changed here? ::: src/placeBubble.js @@ +201,3 @@ + for (let i = 0; i < widgets.length; i++) { + this._expandedContent.remove(widgets[i]); + } What is this for? And if this is needed: this._expandedContent.get_children().forEach(function(child) { child.destroy(); });
Review of attachment 320661 [details] [review]: ::: src/placeBubble.js @@ +201,3 @@ + for (let i = 0; i < widgets.length; i++) { + this._expandedContent.remove(widgets[i]); + } I realized that after the change to use the expanded area, when re-populating the place bubble after a successful OSM editing, we would attach stuff to the expanded grid when there are already stuff in there, the behavior in this case is undefined for GtkGrid. And at some point I got the Wikipedia and Website labels ovewriting each other… Good point about using forEach() instead. Might as well rewrite the loop for the _boxContent the same way.
Review of attachment 320661 [details] [review]: ::: src/place.js @@ +282,3 @@ } else return null; +}; Aha, there was a whitespace change at EOF. I'll fix that… :-)
Created attachment 320757 [details] [review] osmEdit: Add support for editing website Also show a website link in the expanded place area when available. And fix a bug where the expanded area wasn't cleared properly when successfully updating an OSM object, when we're at it.
I actually found another fallout-bug from the change to use the expanded area, now the _boxContentArea doesn't contain the place name label anymore, and the old code removed all but the first row, which resulted now in that the postal code row was retained (and duplicated) after editing an object. So, fixed that as well (and rewrote that loop into a forEach lambda invocation).
Attachment 320757 [details] pushed as 7a7b9fa - osmEdit: Add support for editing website