-
Notifications
You must be signed in to change notification settings - Fork 0
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
Use initialOptions
to save & restore view state on navigation
#7
Conversation
3e21e3a
to
58f5d96
Compare
// Watch id & file together: | ||
this.$watch( | ||
() => ({ | ||
id: this.id ?? undefined, // (treat null as undefined) | ||
file: this.file ?? undefined | ||
}), | ||
async ({ id }, old) => { | ||
// update the query when the id or file change | ||
this.updateQuery() | ||
// refresh the file list when the id changes | ||
if (id !== old?.id) { | ||
await this.updateLogFileList() | ||
} | ||
}, | ||
{ immediate: true } | ||
) |
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.
Rejigged the watchers for file
and id
to prevent race conditions. The usual order of events now is (for the example of opening the workflow log):
-
id
gets set for the first time to'~user/workflow'
.file
is nullish.updateQuery()
gets called because theid
changed, but becausefile
is nullish it doesn't really do anythingLines 429 to 431 in 58f5d96
if (!this.file || !this.id) { this.query = null return updateLogFileList()
gets called because theid
changed
-
file
has been set to the default scheduler log byupdateLogFileList()
updateQuery()
gets called again becausefile
changed, and causes the log file to displayupdateLogFileList()
doesn't get called becauseid
is unchanged
if (this.file && !(this.file in logFiles)) { | ||
if (this.file && !logFiles.includes(this.file)) { |
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.
x in arr
does not do what you think it does!
export const useInitialOptions = (name, { props, emit }) => { | ||
const _ref = ref(props.initialOptions[name]) | ||
watch( | ||
_ref, | ||
(val, old) => emit( | ||
updateInitialOptionsEvent, | ||
{ ...props.initialOptions, [name]: val } | ||
), | ||
{ immediate: true, deep: true } | ||
) | ||
return _ref | ||
} |
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.
This is similar to using a writeable computed property e.g.
computed({
get: () => props.initialOptions[name],
set: (val) => emit(
updateInitialOptionsEvent,
{ ...props.initialOptions, [name]: val }
)
})
but that seemed to lead to the ref value not updating until the next tick? Maybe the watcher approach works because it is backed up by a concrete ref? Not sure
58f5d96
to
49b7b49
Compare
49b7b49
to
90fcb77
Compare
This is saved along with the Lumino layout for restoring after navigation. Implemented for the log view only so far.
90fcb77
to
0152228
Compare
Re-opened as cylc#1688 |
Secondary PR against cylc#1664
The view state (filters, inputs, toggles etc.) can now be saved with the Lumino tab layout by using a prop called
initialOptions
.This is only implemented for the log view for now.