Skip to content

Commit

Permalink
fix: Hide search result checkboxes instead of disabling them (#6568)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
fkling authored Jan 9, 2025
1 parent 3f908ae commit 2fe91f9
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 41 deletions.
80 changes: 42 additions & 38 deletions vscode/webviews/chat/cells/messageCell/assistant/SearchResults.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -277,46 +277,50 @@ export const SearchResults = ({
</>
)}
<div className="tw-flex tw-items-center tw-gap-4">
<Label
htmlFor="search-results.select-all"
className={styles.searchResultsHeaderLabel}
>
Add to context:
</Label>
<input
type="checkbox"
id="search-results.select-all"
checked={
selectedFollowUpResults.size === resultsToShow.length
}
disabled={!enableContextSelection}
onChange={event => {
const checked = event.target.checked

telemetryRecorder.recordEvent(
'onebox.results',
checked ? 'selectAll' : 'deselectAll',
{
billingMetadata: {
product: 'cody',
category: 'billable',
},
{enableContextSelection && (
<>
<Label
htmlFor="search-results.select-all"
className={styles.searchResultsHeaderLabel}
>
Add to context:
</Label>
<input
type="checkbox"
id="search-results.select-all"
checked={
selectedFollowUpResults.size ===
resultsToShow.length
}
)
onChange={event => {
const checked = event.target.checked

if (checked) {
updateSelectedFollowUpResults({
type: 'add',
results: resultsToShow,
})
} else {
updateSelectedFollowUpResults({
type: 'init',
results: [],
})
}
}}
/>
telemetryRecorder.recordEvent(
'onebox.results',
checked ? 'selectAll' : 'deselectAll',
{
billingMetadata: {
product: 'cody',
category: 'billable',
},
}
)

if (checked) {
updateSelectedFollowUpResults({
type: 'add',
results: resultsToShow,
})
} else {
updateSelectedFollowUpResults({
type: 'init',
results: [],
})
}
}}
/>
</>
)}
</div>
</div>
</div>
Expand Down
5 changes: 2 additions & 3 deletions vscode/webviews/components/codeSnippet/CodeSnippet.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -204,19 +204,18 @@ export const FileMatchSearchResult: FC<PropsWithChildren<FileMatchSearchResultPr
triggerOnce: true,
})

const actions = (
const actions = onSelectForContext ? (
<div>
<input
type="checkbox"
id="search-results.select-all"
checked={selectedForContext}
disabled={!onSelectForContext}
onChange={event => {
onSelectForContext?.(event.target.checked, result)
}}
/>
</div>
)
) : null

return (
<ResultContainer
Expand Down

0 comments on commit 2fe91f9

Please sign in to comment.