chore: refactor schedule banner #4274
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
presleyp
requested review from
BrunoQuaresma
and removed request for
a team
| return <FullScreenLoader /> | ||
| } else if (!template) { | ||
| return <FullScreenLoader /> | ||
| } else if (workspace && workspaceState.matches("ready")) { |
There was a problem hiding this comment.
Is not something we could use the Choose one component?
| // }, | ||
| // clearScheduleBannerRef: assign({ | ||
| // scheduleBannerRef: (_) => undefined | ||
| // }), |
There was a problem hiding this comment.
Should we remove these?
There was a problem hiding this comment.
yes thank you! Makes me realize - I tried both ways, spawning and invoking, and I wrote that I spawned but I really invoked! Haha, you can see why, invoking requires less code.
BrunoQuaresma
approved these changes
Sep 30, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
deadline, which comes fromworkspace. But the workspace page machine is huge. So instead of inlining the schedule banner machine inside the workspace page machine, Iinvokeit. This requires breaking the workspace page component into two components, because you can use a hook (to access the child machine) inside a conditional (which ensures the child machine has been invoked).I notice there's a pre-existing bug where we show a success message even if you try to increase (or decrease) beyond the max (or min) and so no change takes effect; something to fix later.