Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

search ui: FileSearchResult refactor#42631

Merged
novoselrok merged 7 commits into
rn/result-container-refactorfrom
rn/file-search-result-refactor
Oct 10, 2022
Merged

search ui: FileSearchResult refactor#42631
novoselrok merged 7 commits into
rn/result-container-refactorfrom
rn/file-search-result-refactor

Conversation

@novoselrok

@novoselrok novoselrok commented Oct 6, 2022

Copy link
Copy Markdown
Contributor

Part of https://github.com/sourcegraph/sourcegraph/issues/42159

Summary

  • Renames FileSearchResult to FileContentSearchResult
  • FileContentSearchResult uses the new ResultContainer
  • Expand/collapse logic is moved to FileContentSearchResult
  • Removed hunks support on the client

Test plan

  • Everything should be covered by tests, no new functionality added.

App preview:

Check out the client app preview documentation to learn more.

@cla-bot cla-bot Bot added the cla-signed label Oct 6, 2022
@novoselrok novoselrok marked this pull request as ready for review October 6, 2022 12:30
@sourcegraph-bot

sourcegraph-bot commented Oct 6, 2022

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff e7a50d0...3d7622d.

Notify File(s)
@fkling client/search-ui/src/components/FileContentSearchResult.test.tsx
client/search-ui/src/components/FileContentSearchResult.tsx
client/search-ui/src/components/FileMatchChildren.tsx
client/search-ui/src/components/FilePathSearchResult.tsx
client/search-ui/src/components/FileSearchResult.test.tsx
client/search-ui/src/components/FileSearchResult.tsx
client/search-ui/src/components/ResultContainer.module.scss
client/search-ui/src/components/ResultContainer.tsx
client/search-ui/src/components/SearchResult.module.scss
client/search-ui/src/components/SymbolSearchResult.tsx
client/search-ui/src/components/index.ts
client/search-ui/src/results/StreamingSearchResultsList.tsx
client/web/src/search/results/StreamingSearchResults.test.tsx
@limitedmage client/search-ui/src/components/FileContentSearchResult.test.tsx
client/search-ui/src/components/FileContentSearchResult.tsx
client/search-ui/src/components/FileMatchChildren.tsx
client/search-ui/src/components/FilePathSearchResult.tsx
client/search-ui/src/components/FileSearchResult.test.tsx
client/search-ui/src/components/FileSearchResult.tsx
client/search-ui/src/components/ResultContainer.module.scss
client/search-ui/src/components/ResultContainer.tsx
client/search-ui/src/components/SearchResult.module.scss
client/search-ui/src/components/SymbolSearchResult.tsx
client/search-ui/src/components/index.ts
client/search-ui/src/results/StreamingSearchResultsList.tsx
client/web/src/integration/search.test.ts
client/web/src/search/results/StreamingSearchResults.test.tsx

@novoselrok novoselrok requested review from a team and limitedmage October 6, 2022 13:05

@limitedmage limitedmage left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great refactor!!

}
}

.header-description {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do these styles have to be removed from ResultContainer.module.scss? (I'm not seeing that file in this PR?)

</Component>
)
}
}) as ForwardReferenceExoticComponent<React.ElementType, React.PropsWithChildren<ResultContainerProps>>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: can we move the type definition next to the variable declaration (eg. export const ResultContainer: ForwardReferenceExoticComponent<React.ElementType, React.PropsWithChildren<ResultContainerProps>> = ...) if it's for sure needed?

@novoselrok novoselrok merged commit a46e742 into rn/result-container-refactor Oct 10, 2022
@novoselrok novoselrok deleted the rn/file-search-result-refactor branch October 10, 2022 06:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants