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

[AiLab] pending delete spinner and show model list without freshly deleted model #40064

Merged
merged 1 commit into from Apr 20, 2021

Conversation

Erin007
Copy link
Contributor

@Erin007 Erin007 commented Apr 14, 2021

Follow up to address #39924 (comment)

  • State to track deletion pending status
  • Use that state to show spinner and pending message in delete button
  • On successful deletion, return to model manager dialog and show updated model list
Screen.Recording.2021-04-13.at.8.45.43.PM.mov

@breville
Copy link
Member

Just out of curiosity: in the video, the initial dataset doesn't have a delete button. Is that expected?

});
} else {
this.setState({confirmDialogOpen: false});
this.setState({confirmDialogOpen: false, isDeletePending: false});
this.getModelList();
Copy link
Member

Choose a reason for hiding this comment

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

Should we show a spinner while isModelListPending is true as well? (If not, should we keep isDeletePendingastrue` until the model list reload completes?

(In theory we could avoid a fresh retrieval of models by just adjusting our local list, but I actually like the idea here of getting a fresh set of information.)

@Erin007
Copy link
Contributor Author

Erin007 commented Apr 14, 2021 via email

@breville
Copy link
Member

After going through the levels, I think we should also show a spinner rather than an empty dropdown when loading the model list when we first see this dialog.

@Erin007
Copy link
Contributor Author

Erin007 commented Apr 20, 2021

show a spinner rather than an empty dropdown when loading the model list when we first see this dialog.

https://trello.com/c/LxDYLahQ/177-show-a-spinner-rather-than-an-empty-dropdown-when-loading-the-model-list-in-the-model-manager tracking as follow up

@Erin007 Erin007 merged commit e70d598 into staging Apr 20, 2021
@Erin007 Erin007 deleted the delete-model-spinner branch April 20, 2021 14:02
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.

None yet

2 participants