Skip to content
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

Disable create on submit #186

Merged
merged 5 commits into from
Oct 24, 2022

Conversation

JasonWeill
Copy link
Collaborator

@JasonWeill JasonWeill commented Oct 22, 2022

Fixes #179. On submission, replaces the "Create" and "Cancel" buttons with a progress indicator. (Clicking "Cancel" after job creation has started will not actually attempt to cancel the job submission request.) If the job succeeds, the user is brought to the list page. If the job fails, the user remains on the create page and the buttons become visible again.

In the video below, the "Create" handler has been replaced with a stub that introduces a 5-second delay and resolves the promise only if the job name contains ok. It does not actually create a job.

Screen.Recording.2022-10-22.at.12.56.02.PM.mov

@github-actions
Copy link
Contributor

Binder 👈 Launch a Binder on branch jweill-aws/jupyter-scheduler/disable-create-on-submit

@JasonWeill JasonWeill added the enhancement New feature or request label Oct 22, 2022
@JasonWeill
Copy link
Collaborator Author

Updates PR to hide both "Cancel" and "Create", since if create takes a long time, a user might try to click "Cancel," but this will not actually cancel the job creation request.

Copy link
Contributor

@ellisonbg ellisonbg left a comment

Choose a reason for hiding this comment

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

Overall this looks good. I think it does make sense to remove the Cancel and Create buttons. A few minor comments:

  • I think we should put a small descriptive text next to the spinner. Something like "Creating job..." or "Creating job definition...".
  • What happens if the user closes the panel while the job is being created and job creation fails? I guess there won't be any record of that job? We should probably open an issue to discuss how to handle this case.

@JasonWeill
Copy link
Collaborator Author

@ellisonbg Added descriptive text to display beside the spinner. Opened #190 to handle asynchronous job creation errors.

@JasonWeill JasonWeill force-pushed the disable-create-on-submit branch from 4259228 to 1c31535 Compare October 24, 2022 16:17
Copy link
Collaborator

@andrii-i andrii-i left a comment

Choose a reason for hiding this comment

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

Looks and works well, significantly improves usability.

I'd consider putting spinner in the middle of the screen to be in line with how I have usually seen it used vs right but this is a subjective preference.

@dlqqq
Copy link
Collaborator

dlqqq commented Oct 24, 2022

Why not just having the loading spinner render inside the "Create" button itself and then disable both the create and cancel buttons? This would allow you to keep roughly the same layout and avoid confusing the user by "flashing" a UI change.

@JasonWeill JasonWeill merged commit aa8a89b into jupyter-server:main Oct 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Indicate that a job is being created after the user clicks "Create"
4 participants