Modify

Opened 13 days ago

Closed 12 days ago

Last modified 11 days 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 13 days ago.
v1-0002-SlippyMapBBoxChooser-redesign-SourceButton-using-a-r.patch (15.6 KB) - added by ris 13 days ago.
v1-0003-SlippyMapBBoxChooser-maintain-state-of-showDownloadA.patch (3.0 KB) - added by ris 13 days ago.
v1_download.png (29.0 KB) - added by ris 13 days ago.
Download dialog with menu open (patch v1)
v1_minimap_closed.png (54.1 KB) - added by ris 13 days ago.
Small minimap with menu closed (patch v1)
v1_minimap_open.png (52.3 KB) - added by ris 13 days ago.
Small minimap with menu open (patch v1)
minimap_plus_button.png (19.6 KB) - added by Klumbumbus 13 days ago.
v2-0001-add-PopupMenuButton-a-button-for-triggering-the-appe.patch (4.4 KB) - added by ris 12 days ago.
v2-0002-SlippyMapBBoxChooser-redesign-SourceButton-using-a-r.patch (17.0 KB) - added by ris 12 days ago.

Download all attachments as: .zip

Change History (27)

comment:1 Changed 13 days ago by ris

I'll also attach some screenshots...

Changed 13 days ago by ris

Attachment: v1_download.png added

Download dialog with menu open (patch v1)

Changed 13 days ago by ris

Attachment: v1_minimap_closed.png added

Small minimap with menu closed (patch v1)

Changed 13 days ago by ris

Attachment: v1_minimap_open.png added

Small minimap with menu open (patch v1)

comment:2 Changed 13 days ago by ris

Download dialog with menu open (patch v1) Small minimap with menu closed (patch v1) Small minimap with menu open (patch v1)

comment:3 Changed 13 days ago by Klumbumbus

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

comment:4 Changed 13 days ago by Klumbumbus

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 13 days ago by Klumbumbus (previous) (diff)

Changed 13 days ago by Klumbumbus

Attachment: minimap_plus_button.png added

comment:5 Changed 13 days ago by ris

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 Changed 13 days ago by bastiK

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 Changed 13 days ago by Don-vip

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

comment:8 Changed 13 days ago by ris

Please add javadoc to all public methods (including constructors)

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

comment:9 Changed 13 days ago by ris

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

comment:10 in reply to:  8 Changed 13 days ago by bastiK

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 Changed 12 days ago by ris

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

comment:12 Changed 12 days ago by bastiK

Resolution: fixed
Status: newclosed

In 12955/josm:

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

comment:13 Changed 12 days ago by bastiK

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

comment:14 Changed 12 days ago by Don-vip

In 12962/josm:

see #15414 - fix javadoc warnings

comment:15 Changed 11 days ago by Klumbumbus

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 Changed 11 days ago by ris

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 Changed 11 days ago by ris

(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.

comment:18 in reply to:  16 Changed 11 days ago by Klumbumbus

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.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.