Skip to content

Changes default to download in background and gives clearer description. #7

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Changes default to download in background and gives clearer description. #7

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions src/org/openstreetmap/josm/plugins/continuosDownload/DownloadPreference.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public class DownloadPreference extends DefaultTabPreferenceSetting implements D
private final JTextField extraDownload = new JTextField(4);
private final JTextField maxArea = new JTextField(4);
private final JComboBox<String> strategy = new JComboBox<>();
private final JCheckBox quietDownload = new JCheckBox(tr("Supress the default modal progress monitor when downloading."));
private final JCheckBox quietDownload = new JCheckBox(tr("Download data in background."));

private final Map<PreferenceTabbedPane, JPanel> guiPanes = new HashMap<>();
/**
Expand Down Expand Up @@ -97,10 +97,8 @@ public void addGui(PreferenceTabbedPane gui) {
panel.add(strategy, GBC.eol().fill(GridBagConstraints.HORIZONTAL).insets(5, 0, 0, 5));

// quietDownload
quietDownload.setSelected(Config.getPref().getBoolean("plugin.continuos_download.quiet_download", false));
quietDownload.setToolTipText(tr("Suppress the progress monitor that is shown when downloading. If"
+ " this option is selected there is no indication that something is being done, and no way to"
+ " cancel the download."));
quietDownload.setSelected(Config.getPref().getBoolean("plugin.continuos_download.quiet_download", true));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know about leaving setting this as the default -- I have seen instances where the download queue can get very large very quickly, and prevent other background tasks from executing (for all intents and purposes).

See https://josm.openstreetmap.de/ticket/21485 for an example.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, ideally that would be handled in a "smarter" way. Like, check if queue is too large, if yes, stop adding more downloads to the queue until its length is below a certain threshold. Or look at how ID is handling things—somehow, everything gets loaded into view seamlessly without ever bothering the user with download promts or errors.

But since that is not yet the case, maybe we could at least put the option in the preferences menu without having to enable expert options?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do agree that continuousDownloads needs to be smarter about the queue. iD downloads z14 tiles, IIRC. TBH, downloading z14 shouldn't be too difficult to implement along with a constraint on zoom levels. The queue can get kind of silly when you zoom out and in quickly.

I, personally, am open to putting the option in the preferences menu without having to enable expert mode. But in that case, I do want to see a warning where the user doesn't have to hover over the quietDownload component.

Example:

Potentially Problematic Options (MAY CAUSE JOSM TO LOCK UP):
[ ] Download data in background

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
quietDownload.setSelected(Config.getPref().getBoolean("plugin.continuos_download.quiet_download", true));
quietDownload.setSelected(Config.getPref().getBoolean("plugin.continuos_download.quiet_download", false));

quietDownload.setToolTipText(tr("Downloads the data continuously in the background as you pan and zoom around."));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd really prefer to leave the warning "If this option is selected there is no indication that something is being done, and no way to cancel the download.".

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. That could certainly be left in the tooltip. Better yet would be adding a download progress bar in the bottom row of the program, akin to the one in the old Internet Explorer.

When I tested, the error message that an area was a too big request for the server still came up when quiet_download was turned on, so maybe this isn't even an issue? How often would it be necessary to cancel the download rather than waiting a few seconds for the queue to clear up or for the server to throw an error?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It kind of depends -- I'd much rather not depend upon the queue clearing up -- despite trying to do parallel downloads, the plugin actually does all the downloads in a single thread. I've fixed that in PR #8 , along with automatically stopping downloads when too many nodes are in the area.

The error will still occur with quiet download enabled, but the user might have done enough panning around that they get into what feels like an endless loop of closing those dialogs.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In other words, let us keep the default at false for plugin.continuos_download.quiet_download, and after this PR and #8 are merged, wait a few months to see if anyone sees negative effects with quiet_download enabled. At that point, I'd be OK with enabling quiet_download by default.

Copy link
Author

@Hidooo Hidooo Mar 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error will still occur with quiet download enabled, but the user might have done enough panning around that they get into what feels like an endless loop of closing those dialogs.

That's nothing new to JOSM, unfortunately. But should probably be avoided if possible :)

In other words, let us keep the default at false for plugin.continuos_download.quiet_download, and after this PR and #8 are merged, wait a few months to see if anyone sees negative effects with quiet_download enabled. At that point, I'd be OK with enabling quiet_download by default.

Sounds good. 👍

panel.add(quietDownload, GBC.eol().insets(0, 0, 0, 0));

panel.add(Box.createVerticalGlue(), GBC.eol().fill(GridBagConstraints.VERTICAL));
Expand Down