-
Notifications
You must be signed in to change notification settings - Fork 231
[PoC] Add option to hide the Cancel button of jobs in the progress view #2823
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
Conversation
For long-running jobs that can't be reliably cancelled, it should be possible to hide the Cancel button, so that the job can still be used to provide feedback to the user without having to worry about the potential cancellation in the progress monitor. This behavior is currently controlled via a property that needs to be set for the job before it is scheduled. But it might also be worthwhile to add this API directly to the Job class. Note: In order to ensure an equal width for cancellable and non-cancellable jobs, the button is created, but simply turned invisible. This should also make it easier to dynamically hide/show the button, if such a functionality is needed at a later stage. Relates to: - https://bugs.eclipse.org/bugs/show_bug.cgi?id=155479 - https://bugs.eclipse.org/bugs/show_bug.cgi?id=102343
| cancellableField.setText("Cancellable"); //$NON-NLS-1$ | ||
| cancellableField.setToolTipText("Whether the job can be cancelled by the user"); //$NON-NLS-1$ | ||
| cancellableField.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, false)); | ||
| cancellableField.setSelection(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tried to run this example but I really have not the slightest clue how one is supposed to load those E4 views... The E4 editor is there but the E4 job/progress view just refuses to be loaded.
| * | ||
| * @since 3.135 | ||
| */ | ||
| QualifiedName CANCELLABLE = new QualifiedName(IProgressConstants.PROPERTY_PREFIX, "cancellable"); //$NON-NLS-1$ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just a placeholder right now to get things working without modifying the platform. I think for visiblity, it is much better to add the isCancellable() and setCancellable() methods directly to the Job class.
| for (JobInfo jobInfo : getJobInfos()) { | ||
| // Hide cancel button if job not cancellable | ||
| if (!jobInfo.isCancellable()) { | ||
| actionBar.setVisible(false); | ||
| return; | ||
| } | ||
| } | ||
| actionBar.setVisible(true); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The action bar only contains the Cancel button. So it should be safe to make the tool-bar invisible, given that this isn't possible for the tool-item.
|
Just to summarize the general use case: You have a long running job that can't be safely cancelled. But you still want to show the user that something is happening. Currently there is no way to hide the cancel button, meaning the only option is to ignore that the user might or might not have clicked it. This then leads to confusion as the user doesn't understand why the job keeps running, even though they requested the cancellation. |
Can you give an example? In the worst case the user will simply kill the Eclipse instance and complain that "some job" has blocked forever. So maybe it would better be the case that pressing the button allows some interception e.g showing a Dialog to ask if the user really wants it (like with save or exit the workbench) at least then one can show a message why it "can't be safely cancelled". |
|
I think it's kind of horrible to have a job you can't cancel. Of course if the button doesn't work, that's not nice either. I don't know if the framework should encourage support. If it's a system job, it won't show at all... |
Refreshing the workspace, updating the decorations, persisting metadata. Anything where an interruption would leave the workspace in an inconsistent/incomplete state. But at least in this specific case, we have a long running job with unknown progress that is calculating the input for a tree viewer. This job dynamically spawns other jobs where we do know the amount of work to give the user feedback that something is indeed happening. Obviously, you can't really cancel the dependent jobs without cancelling the main job. Of course you can handle this with listeners and whatnot, but the user doesn't really see the connection between them.
As opposed to the user killing the application because they clicked on the Cancel button and nothing happened? From my experience, users primarily kill an application on a) UI freezes and less frequently due to b) a lack of feedback.
But what's the alternative? Those monolithic jobs won't go away, regardless of whether this is supported by the framework. So the options are to ignore the cancel button, make a makeshift Job API without cancel button or run them in a background thread without any user feedback. All of which I think are arguably worse.
They in-fact do. They're just hidden by default. |
Could stop after refreshing the current resource / project ...
Could stop after the current decoration
Maybe not the best for a (user) job
I always would like to see that the View would show Job Groups and I can cancel a whole group... in any case I don't see that the operation is not cancelable (unless you using blocked waiting for all jobs), but at best would just cancel all pending jobs. |
I already suggested that cancel a job should interrupt the current thread (but it was not seen useful) unless that will happen a job must react to cancellation sooner or later. It would still be good to have a |
|
I agree that a job that can't be canceled is a really bad thing. That being said, I can also imagine that there exist cases where the partial completion of some task might lead to an inconsistent/broken/bad state. I expect though that every example one might conjure up can be shot down with "so redesign it better" or "do it differently" as we see above... |
|
Then let me just close things with saying that sadly "simply redesigning everything" is usually easer said than done. I appreciate everyones feedback and close this PR. |
|
My intent was NOT for you to close this but rather to point out that there are (may well be) situations where one simply cannot cancel without some ocean-boiling excise to redesign the entire almost-impossible-to-cancel processing. So if you have such a scenario, and clearly others have such a scenario via the bugzillas, then not providing a cancel button seems reasonable to me. Of course that's less than ideal; point made and not worth repeating. The point of my comment was to avoid repeated attempts to come up with an example scenario and having each one shot down with effectively the same argument. In any case, we are a community and the opinions of two people are just the opinions of two people where your opinion has equal weight. |
|
I'm going to reopen this for you. If you don't want to proceed you can of course close it. |
|
Then perhaps let me rephrase my initial response: I agree that having a non-cancellable job is undesirable, but the sad truth is that they exist in the wild and are usually not something that can be easily refactored. However, I also see how simply hiding the cancel button might treat the symptom but it doesn't really solve the underlying conceptual problems. And if used improperly this also easily leads to situations where jobs are uncancellable for no apparent reason, which would then only lead to more problems (e.g. termination of the application by the user, as mentioned above). While I still believe that this is a genuine use case, doing it like this is probably not the way to go. Perhaps it makes more sense to think of a better way to visualize "related" jobs in the progress view? For example via a tree structure with a cancellable job as "parent" and uncancellable jobs as "children"? I have to think about whether this really covers our use case, but at the very least this is then not something that is addressed by this PR, so I don't believe it's worth pursuing this approach. |
If we really want to do that (because of the sad truth) we could improve UX by disabling the cancel button instead of hiding it. |


For long-running jobs that can't be reliably cancelled, it should be possible to hide the Cancel button, so that the job can still be used to provide feedback to the user without having to worry about the potential cancellation in the progress monitor.
This behavior is currently controlled via a property that needs to be set for the job before it is scheduled. But it might also be worthwhile to add this API directly to the Job class.
Note: In order to ensure an equal width for cancellable and non-cancellable jobs, the button is created, but simply turned invisible. This should also make it easier to dynamically hide/show the button, if such a functionality is needed at a later stage.
Relates to: