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.

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

Conversation

Hidooo
Copy link

@Hidooo Hidooo commented Feb 28, 2022

Changes the default to work the same as all modern programs would, without popping up endless modal boxes as the user uses the program. If the user wants to see the download progress with every pan and zoom action, the option should at least be worded so that a layman can understand what it means. I hope this change makes the checkbox easier to understand.

Changes the default to work the same as all modern programs would, without popping up endless modal boxes as the user uses the program. If the user wants to see the download progress with every pan and zoom action, the option should at least be worded so that a layman can understand what it means. I hope this change makes the checkbox easier to understand.
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));

+ " 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));
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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants