-
Notifications
You must be signed in to change notification settings - Fork 47
ScopesVariable: Add flag for emitting on initial load #1239
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
base: main
Are you sure you want to change the base?
Conversation
|
||
// Only update scopes value state when loading is false and the scopes have changed | ||
if (!loading && (scopesHaveChanged || newScopes.length === 0)) { | ||
// Emit on initial load to ensure that the variable is updated when the scopes are empty |
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.
Previously, this would cause a change event every time some url reloading would happen.
what do you mean by 'url reloading' ?
why would scopesHaveChanged be true and need to be ignored?
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.
ah, is it the scenario where newScopes.length === 0?
hm... why is state updated with new empty scopes array on every url update?
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.
It was noticed when switching on/off edit mode in dashboards, with empty set of scopes. I would assume that the URL change it causes also makes the scopes update?
Regardless, I think newScopes.length === 0
is weird check that doesn't really do what it is supposed to. scopesHaveChanged is not true for the initial load.
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.
@tskarhed but why is stateObservable emitting a new scope value when going into/out of edit mode?
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.
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.
@tskarhed but why is stateObservable emitting a new scope value when going into/out of edit mode?
I'll see if I can look further into it next week.
@torkelo apparently this only happens when no scope is selected. @tskarhed is there any special handling for such case with empty scopes? I don't understand why this problem only occurs with scopes being empty.
Do you mean the emitting of stateObservable when there is no reason to?
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.
Do you mean the emitting of stateObservable when there is no reason to?
yes, exactly
Previously, this would cause a change event every time some url reloading would happen.