getodk / getodk/central

SubmissionList: modelValue of Multiselect can change while options are shown

Open
#1,273 1 comment 0 reactions 0 assignees View on GitHub
frontend
Dominant language
JavaScript
Stars
226
Forks
235
Avg merge
2d 22h
Merged PRs (30d)
19

Description

The `Multiselect` component assumes that its `modelValue` prop will not change while the list of options (the dropdown) is shown. That seems like it should be a fair assumption, yet it is possible for it to be violated for `SubmissionFiltersSubmitter` and `SubmissionFiltersReviewState`, two `Multiselect` components in `SubmissionList`. If the user clicks either `Multiselect`, then changes the corresponding query parameter (`submitter` or `reviewState`) while the options are shown — manually, by pressing the back button, etc. — then that will violate the assumption. The resulting state isn't well-defined, but it does look like things go wrong:

- Navigate to the Submissions page.
- Use the review state filter to filter for approved submissions. That will change the query string to `?reviewState='approved'`.
- Click the review state filter again. While its options are shown, change the query string to `?reviewState='approved'&reviewState='rejected'`.
- Observe that nothing has changed in the list of options: Rejected is still not selected.
- That alone might be reasonable behavior. But now do select Rejected and close the list of options.
- `SubmissionFiltersReviewState` emits an `update:modelValue` event though its `modelValue` hasn't actually changed. You would think that that would cause the router to navigate to the same location, resulting in a router error.
- However, what happens is more interesting: only Rejected ends up being selected, not Approved. I think that's because `selected` is cleared as soon as the `modelValue` changes.

I think there are two main ways that we could address this issue. (I'm thinking that this isn't something that we need to do for v2022.3.)

First, we could prevent navigation while the list of options of either `Multiselect` is shown. I don't think it makes sense for `Multiselect` itself to implement that logic, but if we had `Multiselect` emit `shown` and `hidden` events, then `SubmissionFiltersSubmitter` and `SubmissionFiltersReviewState` could watch for those and prevent navigation if there has been a `shown` event without a `hidden` event.

Alternatively, we could change `Multiselect` so that it allows `modelValue` to change while the list of options is shown. We would have to decide what the desired behavior is in that case (should the selections update immediately to match the new `modelValue`? what if the user has already made changes?). We might end up needing to patch `selected` and/or `changes` as soon as `modelValue` changes.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.