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

fix(sweep): wait for scheduler start/stop #529

Merged
merged 1 commit into from
Oct 3, 2022

Conversation

theborakompanioni
Copy link
Collaborator

@theborakompanioni theborakompanioni commented Oct 3, 2022

Closes #267.

Before this PR, the Sweep screen did not wait for an operation to start/stop and assumed the operation will succeed.
After this PR, the screen will show a loading indicator till the service has been successfully started/stopped.

@theborakompanioni theborakompanioni added bug Something isn't working UI/UX Issue related to cosmetics, design, or user experience labels Oct 3, 2022
@theborakompanioni theborakompanioni self-assigned this Oct 3, 2022
@theborakompanioni theborakompanioni marked this pull request as ready for review October 3, 2022 13:05
@theborakompanioni theborakompanioni requested a review from a team October 3, 2022 15:49
Copy link
Contributor

@dergigi dergigi left a comment

Choose a reason for hiding this comment

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

Looks good to me, tACK ✅

}

return (
<>
<PageTitle title={t('scheduler.title')} subtitle={t('scheduler.subtitle')} />
{alert && <rb.Alert variant={alert.variant}>{alert.message}</rb.Alert>}

{isLoading ? (
{isLoading || isWaitingSchedulerStart || isWaitingSchedulerStop ? (
<rb.Placeholder as="div" animation="wave">
Copy link
Contributor

Choose a reason for hiding this comment

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

Unrelated to this PR, but I thought I'd mention it anyway: I'm not sure how animation="wave" should look like, but this is how the loading state looks for me when starting/stopping the scheduler:

Screenshot 2022-10-03 at 18 27 31

Independent of the changes in this PR, and independent of browser (Firefox and Chromium).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, this is "expected". Should resemble either the "start"/"stop" button or the "Service already running"-alert.
Any inputs highly appreciated.

@dergigi dergigi merged commit 509a15e into master Oct 3, 2022
@dergigi dergigi deleted the wait-for-scheduler-start branch October 3, 2022 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working UI/UX Issue related to cosmetics, design, or user experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wait for operation to start successfully on Jam screen
2 participants