Modify

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#15414 closed enhancement (fixed)

[PATCH] Redesign of SlippyMapBBoxChooser's SourceButton

Reported by: ris Owned by: team
Priority: normal Milestone: 17.10
Component: Core Version:
Keywords: jmapviewer slippy source button Cc:

Description

Here's an implementation of the ideas I was sketching out in #15369. This mostly fell out of the need to be able to extend the source button's capabilities to allow e.g. enabling/disabling of "show downloaded area".

Advantages of this approach:

  • Extensibility
  • Potential for improved accessibility
  • Proper functioning with SlippyMapBBoxChooser widgets too small to show full source list
  • Attempts to look a _bit_ like the layer-chooser that might be found in more modern slippy maps like Leaflet

Advantages of old approach:

  • Looks like the layer-chooser in old OpenLayers slippy maps. But these are becoming rarer all the time and a user is less likely to have encountered one nowadays.

In places you'll notice I've tried to make decisions that would make it easier for plugin authors to add their own options to the menu, but of course everywhere I've tried to strike a balance between clean design and potential API breakage.

Patches are against r12940.

Can also browse the patches in github: https://github.com/risicle/josm/compare/576e806f627c648afdf09a9b04138b109e298164...ris-slippy-map-source-button

Attaching patches...

Attachments (9)

v1-0001-add-PopupMenuButton-a-button-for-triggering-the-appe.patch (3.3 KB ) - added by ris 7 years ago.
v1-0002-SlippyMapBBoxChooser-redesign-SourceButton-using-a-r.patch (15.6 KB ) - added by ris 7 years ago.
v1-0003-SlippyMapBBoxChooser-maintain-state-of-showDownloadA.patch (3.0 KB ) - added by ris 7 years ago.
v1_download.png (29.0 KB ) - added by ris 7 years ago.
Download dialog with menu open (patch v1)
v1_minimap_closed.png (54.1 KB ) - added by ris 7 years ago.
Small minimap with menu closed (patch v1)
v1_minimap_open.png (52.3 KB ) - added by ris 7 years ago.
Small minimap with menu open (patch v1)
minimap_plus_button.png (19.6 KB ) - added by Klumbumbus 7 years ago.
v2-0001-add-PopupMenuButton-a-button-for-triggering-the-appe.patch (4.4 KB ) - added by ris 7 years ago.
v2-0002-SlippyMapBBoxChooser-redesign-SourceButton-using-a-r.patch (17.0 KB ) - added by ris 7 years ago.

Download all attachments as: .zip

Change History (27)

comment:1 by ris, 7 years ago

I'll also attach some screenshots...

by ris, 7 years ago

Attachment: v1_download.png added

Download dialog with menu open (patch v1)

by ris, 7 years ago

Attachment: v1_minimap_closed.png added

Small minimap with menu closed (patch v1)

by ris, 7 years ago

Attachment: v1_minimap_open.png added

Small minimap with menu open (patch v1)

comment:3 by Klumbumbus, 7 years ago

Ticket #14705 has been marked as a duplicate of this ticket.

comment:4 by Klumbumbus, 7 years ago

Milestone: 17.10

Would be great in 17.10 to not upset users who don't like the display of the downloaded area, if there are any ;)

Also the layer chooser in the minimap is currently pretty much useless in my case:


Last edited 7 years ago by Klumbumbus (previous) (diff)

by Klumbumbus, 7 years ago

Attachment: minimap_plus_button.png added

comment:5 by ris, 7 years ago

Oh wow that's nasty.

Additions I'm considering but aren't currently included in the patches are:

  • Tooltip on button ("Showing 'Bing aerial imagery': click to select other imagery sources" ?)
  • Actually disable the "Show downloaded area" checkbox when no appropriate layer is available to remove the apparent contradiction of having the option selected, yet no markings.

and then of course, this all needs tests.

comment:6 by bastiK, 7 years ago

Looks good, some minor things:

  • Please add javadoc to all public methods (including constructors) except for those with @Override annotation. Update javadoc to reflect new method signature.
  • No double newline & check unused imports
  • clip does not give component dimensions but the part of the component that was obstructed and needs to be repainted.
  • You can upload a single patch file (I don't mind).

comment:7 by Don-vip, 7 years ago

You can run ant checkstyle to check everything's ok before creating the patch :)

comment:8 by ris, 7 years ago

Please add javadoc to all public methods (including constructors)

For the JButton passthrough ones should I just copy the swing doc strings?

comment:9 by ris, 7 years ago

Ah ok maybe I can use an @see...

in reply to:  8 comment:10 by bastiK, 7 years ago

Replying to ris:

Please add javadoc to all public methods (including constructors)

For the JButton passthrough ones should I just copy the swing doc strings?

The swing doc is copyrighted, so just put something similar.

comment:11 by ris, 7 years ago

I've kept it as two patches because the addition of PopupMenuButton is quite separate from the SlippyMapBBoxChooser stuff.

comment:12 by bastiK, 7 years ago

Resolution: fixed
Status: newclosed

In 12955/josm:

applied #15414 - Redesign of SlippyMapBBoxChooser's SourceButton (patch by ris)

comment:13 by bastiK, 7 years ago

Please observe Jenkins and fix any warnings that you feel responsible for. :)

comment:14 by Don-vip, 7 years ago

In 12962/josm:

see #15414 - fix javadoc warnings

comment:15 by Klumbumbus, 7 years ago

When adding or removing entries in the imagery list in the preferences then the list in the download window automatically updates while the list in the minimap doesn't, even after closing and reopening the minmap. It seems a complete josm restart is required to update the list in the minimap.

comment:16 by ris, 7 years ago

This is as it already was, no?

Yes, it's something I should take a look at and probably write a test for.

comment:17 by ris, 7 years ago

(oh and yes, sorry, the trailing underscores are because Java's symbol ambiguity (function arg vs instance field) scares me. Same goes for my use of this. everywhere.

in reply to:  16 comment:18 by Klumbumbus, 7 years ago

Replying to ris:

This is as it already was, no?

I don't know. I just noticed the behavior while testing the new feature. However this is a minor problem and should be low priority.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.