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

search frontend: render streaming search results #16097

Merged
merged 1 commit into from Nov 24, 2020
Merged

Conversation

limitedmage
Copy link
Contributor

Renders search results for streaming search!
This still uses the GraphQL data types. This will be later changed as @keegancsmith is going to update the API so conversion to GraphQL types is no longer required (primarily adding a string type to each result)

Note that lots of features are still missing here (eg. loading state, footer, etc). This is meant to be the initial work to render results.

There is a bug in storybook tests where not all code excerpts are being rendered: #16095 This doesn't happen in the real webapp.

image

@limitedmage limitedmage requested review from a team November 23, 2020 23:36
@limitedmage limitedmage added this to In progress in Search iterations via automation Nov 23, 2020
@codecov
Copy link

codecov bot commented Nov 23, 2020

Codecov Report

Merging #16097 (b568b5f) into main (dd02c3a) will increase coverage by 0.00%.
The diff coverage is 92.85%.

@@           Coverage Diff           @@
##             main   #16097   +/-   ##
=======================================
  Coverage   52.86%   52.87%           
=======================================
  Files        1645     1645           
  Lines       82314    82328   +14     
  Branches     7248     7341   +93     
=======================================
+ Hits        43517    43532   +15     
+ Misses      34947    34945    -2     
- Partials     3850     3851    +1     
Flag Coverage Δ
go 52.68% <ø> (+<0.01%) ⬆️
integration 28.57% <14.28%> (+<0.01%) ⬆️
storybook 28.24% <92.85%> (+0.14%) ⬆️
typescript 53.35% <92.85%> (+0.02%) ⬆️
unit 35.83% <92.85%> (+0.07%) ⬆️
Impacted Files Coverage Δ
...earch/results/streaming/StreamingSearchResults.tsx 98.03% <92.30%> (-1.97%) ⬇️
client/shared/src/util/searchTestHelpers.ts 94.44% <100.00%> (+0.32%) ⬆️
cmd/repo-updater/repos/types.go 74.44% <0.00%> (+0.24%) ⬆️
client/web/src/components/SearchResult.tsx 75.00% <0.00%> (+6.25%) ⬆️

Copy link
Contributor

@felixfbecker felixfbecker left a comment

Choose a reason for hiding this comment

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

@limitedmage anything in particular you would like non-search frontend devs to look at here?

url: '/github.com/golang/oauth2',
matches: [] as ISearchResultMatch[],
label: { __typename: 'Markdown', text: '[github.com/golang/oauth2](github.com/golang/oauth2)' },
} as IRepository
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you considered using our precise query-specific generated types? Even if you're not running a query but using SSE, you could define a fragment for it (that's not used) and use the type for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we definitely want to use backend-generated types. I'll see how this can be done separately.

@limitedmage
Copy link
Contributor Author

@limitedmage anything in particular you would like non-search frontend devs to look at here?

Mostly any general comments on making sure that I'm doing the right thing in the frontend :) Eg. Maybe I'm not following a pattern correctly or there is a different component I can use to accomplish something I'm doing manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Streaming Search: results should be rendered as they come in
3 participants