Skip to content
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

fix: Hide search result checkboxes instead of disabling them #6568

Merged

Conversation

fkling
Copy link
Contributor

@fkling fkling commented Jan 9, 2025

When a follow up query to a search result was submitted we've disabled
the checkboxes of the previous search result because changing the
selection wouldn't impact the already submitted follow up query (but
also from an implementation perspective we wouldn't actually be able to
target the follow up input).

But because we store which results have been selected in memory only,
rerendering the search results would show all checkboxes again as
selected (albeit disabled).

This might be confusing to the user.

We would have to persist the UI state somewhere/somehow but there is no
good place for this atm. So to (hopefully) reduce the confusion we
simply hide the checkboxes when the user cannot interact with the search
results anymore.

Fixes https://linear.app/sourcegraph/issue/PROD-420/context-boxes-re-select-after-query-submission

Test plan

Manually tested locally via web demo.

When a follow up query to a search result was submitted we've disabled
the checkboxes of the previous search result because changing the
selection wouldn't impact the already submitted follow up query (but
also from an implementation perspective we wouldn't actually be able to
target the follow up input).

But because we store which results have been selected in memory only,
rerendering the search results would show all checkboxes again as
selected (albeit disabled).

This might be confusing to the user.

We would have to persist the UI state somewhere/somehow but there is no
good place for this atm. So to (hopefully) reduce the confusion we
simply hide the checkboxes when the user cannot interact with the search
results anymore.
@fkling
Copy link
Contributor Author

fkling commented Jan 9, 2025

This change is part of the following stack:

Change managed by git-spice.

@fkling fkling requested a review from thenamankumar January 9, 2025 09:05
@fkling fkling self-assigned this Jan 9, 2025
@fkling fkling merged commit 2fe91f9 into main Jan 9, 2025
23 checks passed
@fkling fkling deleted the fkling-prod-420-context-boxes-re-select-after-query-submission branch January 9, 2025 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants