[Patch] Download Dialog: UI cleanup

[bafonins]

Hi everyone! I would like to share the result of my work on the download dialog. The task was to clean the existing codebase and develop new, generic way to add any download sources to JOSM, as well as collect all download related functionality at one place having only a single entry point.

This is how it looks in the normal mode :

The main features from the old dialog are still present, except for the area check. But this can be added later.

This is the view in the expert mode with the Overpass tab opened :

All features from #15057 are present.

This is the view in the expert mode with the OSM tab opened :

As you can see the pane dynamically changes according to the user selected mode.
The new interface allows to embed any external download source easily, even through plugins.
The code for download dialog is cleaner.

Will add the patch a bit later. I would like to hear thoughts about the new UI from people who use JOSM extensively. Any feedback is appreciated.

[Don-vip]

looks very nice! :)

[Klumbumbus]

I like it too.

[bafonins]

Here is the patch.

[bafonins]

It would be great to get some feedback, since the download dialog is pretty important component I it might be the case that I am missing something, or removed something that is not supposed to be removed. Also, remarks on the design are highly appreciated.
Some remarks :
OverpassDownloadAction is removed, since within this patch all download sources reside in the same dialog. It can still be added if necessary by simply extending the download dialog with additional methods to set initial opening tab.
As mentioned earlier, the label for area size check is not present.

[michael2402]

I'd like to merge this today, if there are no other objections ;-).

[stoecker]

Fine, if you fix this: "As mentioned earlier, the label for area size check is not present."

[Don-vip]

no objection, go on :)

[bafonins]

added label to check download area

[michael2402]

In 12652/josm:

Apply #15167: Merge OSM and overpass download dialog. Patch by bafonins

[michael2402]

In 12653/josm:

See #15167: Convert to iterator based for loop.

[michael2402]

In 12654/josm:

See #15167: Fix generics.

[michael2402]

In 12655/josm:

See #15167: Add javadoc, improve code style.

[Don-vip]

Two problems:

1. The vertical size of top panel is initially too important, a lot of space is wasted
2. Refresh problem when the dialog is resized: the whole dialog blinks, this is very disturbing

[Klumbumbus]

It would be useful if every source tab remebers its own position of the separator bar above the map, so it changes automatically when swithing the source tabs as shown on the screenshots in the ticket description.
Right now when you move the separator bar up in the OSM tab and then switch to the Overpass tab you need to move it down again to see something.

[Klumbumbus]

The new interface allows to embed any external download source easily, even through plugins.

Anyone has an idea which additional sources might be available?

[bafonins]

refactored some code, fixed docs, removed redundant code according to Michaels changes, each download source has its own size

[bafonins]

Hm, thats weird. Does not happen to me. On what operating system does it happen?

Replying to Don-vip:

Two problems:

1. The vertical size of top panel is initially too important, a lot of space is wasted
2. Refresh problem when the dialog is resized: the whole dialog blinks, this is very disturbing

[Don-vip]

Windows 10. What's yours?

[Klumbumbus]

Flickering see attachment:flicker.gif​

(The flickering happens to me also with the old overpass window.)

[bafonins]

Oh, that looks awful. Will check it

Replying to Klumbumbus:

Flickering see attachment:flicker.gif​

(The flickering happens to me also with the old overpass window.)

[bafonins]

os x. Will have access to windows machine tomorrow, will check it

Replying to Don-vip:

Windows 10. What's yours?

[michael2402]

Replying to Klumbumbus:

Flickering see attachment:flicker.gif​

This might be a JDK bug. It is not happening on Linux. But I did not find any matching bug report. Which version are you using?

[Klumbumbus]

URL:http://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2017-08-26 15:29:22 +0200 (Sat, 26 Aug 2017)
Build-Date:2017-08-26 14:04:15
Revision:12666
Relative:URL: ^/trunk

Identification: JOSM/1.5 (12666 en) Windows 10 64-Bit
OS Build number: Windows 10 Pro 1703 (15063)
Memory Usage: 1930 MB / 3641 MB (1223 MB allocated, but free)
Java version: 1.8.0_144-b01, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Screen: \Display0 1680x1050
Maximum Screen Size: 1680x1050
VM arguments: [-Djava.security.manager, -Djava.security.policy=file:<java.home>\lib\security\javaws.policy, -DtrustProxy=true, -Djnlpx.home=<java.home>\bin, -Djnlpx.origFilenameArg=C:\Program Files (x86)\josm-latest-mehr-RAM.jnlp, -Djnlpx.remove=false, -Djava.util.Arrays.useLegacyMergeSort=true, -Djnlpx.heapsize=1024m,4096m, -Djnlpx.splashport=52724, -Djnlpx.jvm=<java.home>\bin\javaw.exe]
Dataset consistency test: No problems found

Plugins:
+ AddrInterpolation (33160)
+ DirectDownload (33160)
+ DirectUpload (33182)
+ FastDraw (33182)
+ HouseNumberTaggingTool (33160)
+ Mapillary (v1.5.5)
+ OpeningHoursEditor (33185)
+ ShapeTools (1220)
+ Tracer2 (33004)
+ alignways (33182)
+ apache-commons (33517)
+ apache-http (32699)
+ buildings_tools (33004)
+ contourmerge (1030)
+ editgpx (33004)
+ imagery-xml-bounds (33496)
+ imagery_offset_db (33316)
+ jogl (1.1.0)
+ log4j (32699)
+ measurement (33088)
+ osm-obj-info (1484152384)
+ photo_geotagging (33494)
+ photoadjust (33303)
+ reltoolbox (33530)
+ reverter (33088)
+ tag2link (33382)
+ tageditor (33021)
+ tagging-preset-tester (33004)
+ terracer (33088)
+ turnlanes-tagging (254)
+ turnrestrictions (33537)
+ undelete (33480)
+ utilsplugin2 (33543)
+ wikipedia (33541)

[Klumbumbus]

I updated the docu: Help/Action/Download

[bastiK]

This is with fresh preferences:

Before (12545):

Now (12681):

As you can see, the explanation text is cut off, with strange vertical placement. I would prefer the warning text to go on the bottom (as before), but its no big deal, as long as the top panel is properly sized.

In the "Download from OSM" tab, the JSplitPane is unnecessary, as the size of the top panel is fixed. I would argue, a JTabbedPane with a single tab makes no sense. So in non-expert mode, we could do without it.

The only point that is a blocker for the release is the excessive initial size of the top area. (Sorry for being so critical, it's actually great work.)

Relative:URL: ^/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2017-08-28 00:23:19 +0200 (Mon, 28 Aug 2017)
Revision:12681
Build-Date:2017-08-28 01:33:34
URL:http://josm.openstreetmap.de/svn/trunk

Identification: JOSM/1.5 (12681 en) Linux Ubuntu 17.04
Memory Usage: 371 MB / 1984 MB (168 MB allocated, but free)
Java version: 9-Ubuntu+0-9b161-1, Oracle Corporation, OpenJDK 64-Bit Server VM
Screen: :0.0 1366x768
Maximum Screen Size: 1366x768
Java package: openjdk-9-jre:amd64-9~b161-1
Java ATK Wrapper package: libatk-wrapper-java:all-0.33.3-13
VM arguments: [-Djosm.restart=true, -Djosm.dir.name=JOSM-latest, -Djava.net.useSystemProxies=true, -Djosm.home=pref-josm-l2]

[Don-vip]

The flickering on Windows is blocking too.

[bafonins]

moved the info label in non expert mode.

[bafonins]

The latest patch solves the issue with separating download sources and download selections. By default each download source uses minimum size, but this can be changed by the user.
I agree that JSplitPane is not really needed in the 'Download from OSM' tab, but it might be needed for other download sources and removing/adding it depending on the mode/tab is not so trivial. Finally, it can be ignored by the user if the dialog is sized properly.

Replying to bastiK:

This is with fresh preferences:

Before (12545):

Now (12681):

As you can see, the explanation text is cut off, with strange vertical placement. I would prefer the warning text to go on the bottom (as before), but its no big deal, as long as the top panel is properly sized.

In the "Download from OSM" tab, the JSplitPane is unnecessary, as the size of the top panel is fixed. I would argue, a JTabbedPane with a single tab makes no sense. So in non-expert mode, we could do without it.

The only point that is a blocker for the release is the excessive initial size of the top area. (Sorry for being so critical, it's actually great work.)

Relative:URL: ^/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2017-08-28 00:23:19 +0200 (Mon, 28 Aug 2017)
Revision:12681
Build-Date:2017-08-28 01:33:34
URL:http://josm.openstreetmap.de/svn/trunk

Identification: JOSM/1.5 (12681 en) Linux Ubuntu 17.04
Memory Usage: 371 MB / 1984 MB (168 MB allocated, but free)
Java version: 9-Ubuntu+0-9b161-1, Oracle Corporation, OpenJDK 64-Bit Server VM
Screen: :0.0 1366x768
Maximum Screen Size: 1366x768
Java package: openjdk-9-jre:amd64-9~b161-1
Java ATK Wrapper package: libatk-wrapper-java:all-0.33.3-13
VM arguments: [-Djosm.restart=true, -Djosm.dir.name=JOSM-latest, -Djava.net.useSystemProxies=true, -Djosm.home=pref-josm-l2]

[bafonins]

After some testing on win8 it turned out that the reason that causes flickering is in the overriden paint method in DownloadDialog. The comment for the method is not really descriptive. Any ideas?
If I remove it, the dialog resizes without any problems.

Replying to Don-vip:

The flickering on Windows is blocking too.

[bastiK]

In 12684/josm:

see #15167 - moved the info label in non expert mode (patch by bafonins)

[bastiK]

In 12685/josm:

see #15167, see #6720 - remove workaround that causes flickering on resize (idea by bafonins)

(hopefully, the original problem is no longer present)

[bastiK]

Replying to bafonins:

After some testing on win8 it turned out that the reason that causes flickering is in the overriden paint method in DownloadDialog. The comment for the method is not really descriptive. Any ideas?
If I remove it, the dialog resizes without any problems.

We will have to see, if there are now problems as reported in #6720. In a quick test, it was okay.

[Klumbumbus]

Replying to bastiK:

I would prefer the warning text to go on the bottom (as before)

I think it is better on top as this warning is specific to the source (i.e. download from osm). When you switch to the download from overpass api tab this warning is not relevant.

[Klumbumbus]

(r12690 with fresh preferences)
I think the initial size of the top panel of the download from osm tab is still too high, especially if compared with the old dialog.

[Don-vip]

+1, it could be lowered a little bit. But it's much better now, the flickering issue is gone, and if we manually reduce the size of top panel, it's remembered :)

[bastiK]

Regarding the line

setMinimumSize(new Dimension(450, 115));

in [12684]: I don't think it is a good idea to set the size in pixels. The minimum height should be determined by the layout manager instead.

[Don-vip]

In 12694/josm:

see #15167 - checkstyle

[michael2402]

I was traveling this week so I had not time to pick up with it.

I'll have a look at it tomorrow and fix the sizing of the top panels (so that they use the right initial size).

[Don-vip]

Replying to michael2402:

I'll have a look at it tomorrow

Is it ok for tonight? Only a few minutes remain for August :)

[michael2402]

Replying to Don-vip:

Replying to michael2402:

I'll have a look at it tomorrow

Is it ok for tonight? Only a few minutes remain for August :)

I'm already on it. I'm glad we did not agree on a time zone :D

[bastiK]

Today officially ends 3:30 CEST, when cron kicks off the nightly build.

[michael2402]

In 12705/josm:

See #15167: Make size of the OSM download panel fixed, only allow resizing for overpass.

[michael2402]

In 12706/josm:

See #15167: Store the name of the current selected tab.

This also makes the expert mode tab selection more universal

[stoecker]

Remembering the translated name is not correct. It should remember the original English name instead.

P.S. Our release dates are not made of stone. They are recommendations...

[anonymous]

Simple name of a download source is not translated and is not visible to the user.

Replying to stoecker:

Remembering the translated name is not correct. It should remember the original English name instead.

P.S. Our release dates are not made of stone. They are recommendations...

[stoecker]

Ah ok.

[bafonins]

sorry, forgot to login

Replying to anonymous:

Simple name of a download source is not translated and is not visible to the user.

Replying to stoecker:

Remembering the translated name is not correct. It should remember the original English name instead.

P.S. Our release dates are not made of stone. They are recommendations...

[SanderH]

downloading or uploading

[SanderH]

In version 12707 (and a few versions before) I noticed that the download dialog mentions 'Uploading' instead of 'Downloading'.
Don't know if this deserves a separate ticket or if it's related to this one.

[michael2402]

Replying to SanderH:

In version 12707 (and a few versions before) I noticed that the download dialog mentions 'Uploading' instead of 'Downloading'.
Don't know if this deserves a separate ticket or if it's related to this one.

This message is not really correct, we should change it to Requesting.

[Don-vip]

Where does the string come from? I can find only two occurrences in OsmServerWriter and ProgressOutputStream. Nothing related to download at first glance.

[Don-vip]

OK got it: unexpected side effect of the switch from GET to POST method for Overpass API download in r12596 (#15141). I look into it, thank you for reporting this issue.

[Don-vip]

In 12711/josm:

see #15141, see #15167 - use correct message ("Downloading data" instead of "Uploading data") when downloading data from Overpass API / POST

[bastiK]

Are there any outstanding issues in this ticket (i.e. release critical)?

[michael2402]

None that I have encountered. There is no Plugin API yet to register own sources, but this can wait until someone has a nice idea of what sources to add ;-)

[SanderH]

Replying to michael2402:

There is no Plugin API yet to register own sources, but this can wait until someone has a nice idea of what sources to add ;-)

In The Netherlands we have a plugin (​https://github.com/gidema/josm-ods-bag/tree/0.6-dev) to download building and address information from the government and others may use similar plugins.
I think it would be nice if this kind of plugins could be integrated in the standard download dialog. At least that would give a consistent way of downloading data from multiple sources from one central starting point.

[Klumbumbus]

Replying to michael2402:

of what sources to add ;-)

I don't know if http://overpass.osm.rambler.ru/cgi/xapi? is still active (MirroredDownloadInfo)

[michael2402]

Replying to SanderH:

In The Netherlands we have a plugin (​https://github.com/gidema/josm-ods-bag/tree/0.6-dev) to download building and address information from the government and others may use similar plugins.

That's a nice one to get started.

I posted them an issue. ​https://github.com/gidema/josm-ods-bag/issues/26

[Klumbumbus]

Replying to bastiK:

Are there any outstanding issues in this ticket (i.e. release critical)?

12712 is a nice number for releasing :)

[bastiK]

Replying to michael2402:

None that I have encountered. There is no Plugin API yet to register own sources, but this can wait until someone has a nice idea of what sources to add ;-)

Can be done in a new ticket.

[Don-vip]

In 12733/josm:

see #15167 - sonar - pmd:ImmutableField - Immutable Field