-
Notifications
You must be signed in to change notification settings - Fork 30
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
Simplify loading state and avoid races on start/stopping of instances #645
Conversation
Demo starting at https://lxd-ui-645.demos.haus |
2f490da
to
a231995
Compare
src/context/instanceLoading.tsx
Outdated
const [instanceStates, setInstanceStates] = useState( | ||
new Map<string, LoadingTypes>(), | ||
); | ||
const instanceStates = new Map<string, LoadingTypes>(); |
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.
Does context trigger re-renders if we remove state here? i.e. does changes in the Map object trigger re-renders in children components subscribed to this context?
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.
Good point, this wasn't working properly when going to detail page / list page and back while a state change is in progress. Switched to a different approach just now.
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.
aah nice, by using the callback for setState
calls you are guaranteed to use the correct previous state, nice! 👍
a231995
to
b871fa8
Compare
Signed-off-by: David Edler <david.edler@canonical.com>
b871fa8
to
e0bf928
Compare
LGTM 👍 |
Done
QA