[Search Result]: Add new layout page and file preview panel #58311
Conversation
vovakulikov
force-pushed
the
vk/add-new-search-results-filters
branch
from
9cc534b
to
81bac02
Compare
|
Codenotify: Notifying subscribers in CODENOTIFY files for diff 6ec6c7d...a5845c4.
|
|
Codenotify: Notifying subscribers in OWNERS files for diff 6ec6c7d...a5845c4.
|
There was a problem hiding this comment.
Looks great @vovakulikov. Incredible work!
Not too fussed with design on it as of yet considering it's behind a feature flag.
I'm currently working on search results. I'll add an updated concept for this panel with any ideas I have to the designs in a second pass for this.
Amazing work.
There was a problem hiding this comment.
Implementation seems fine (as always). From a UX perspective I'm worried that with the preview panel open I see even less search results. Also I think it would make more useful if the preview could be opened at a particular search result, not at the top of the file.
But I assume that's somewhere on the roadmap and this is the initial version.
| return !newSearchNavigation ? ( | ||
| <SearchContent | ||
| submittedURLQuery={submittedURLQuery} | ||
| queryState={queryState} | ||
| liveQuery={liveQuery} | ||
| allExpanded={allExpanded} | ||
| searchMode={searchMode} | ||
| trace={!!trace} | ||
| searchContextsEnabled={props.searchContextsEnabled} | ||
| patternType={patternType} | ||
| results={results} | ||
| showAggregationPanel={showAggregationPanel} | ||
| selectedSearchContextSpec={props.selectedSearchContextSpec} | ||
| aggregationUIMode={aggregationUIMode} | ||
| caseSensitive={caseSensitive} | ||
| authenticatedUser={authenticatedUser} | ||
| isSourcegraphDotCom={isSourcegraphDotCom} | ||
| enableRepositoryMetadata={enableRepositoryMetadata} | ||
| options={options} | ||
| codeMonitoringEnabled={codeMonitoringEnabled} | ||
| fetchHighlightedFileLineRanges={props.fetchHighlightedFileLineRanges} | ||
| onNavbarQueryChange={setQueryState} | ||
| onSearchSubmit={handleSidebarSearchSubmit} | ||
| onQuerySubmit={handleSearchAggregationBarClick} | ||
| onExpandAllResultsToggle={onExpandAllResultsToggle} | ||
| onSearchAgain={onSearchAgain} | ||
| onDisableSmartSearch={onDisableSmartSearch} | ||
| onLogSearchResultClick={logSearchResultClicked} | ||
| settingsCascade={props.settingsCascade} | ||
| telemetryService={telemetryService} | ||
| platformContext={platformContext} | ||
| /> | ||
| ) : ( | ||
| <NewSearchContent | ||
| submittedURLQuery={submittedURLQuery} | ||
| queryState={queryState} | ||
| liveQuery={liveQuery} | ||
| allExpanded={allExpanded} | ||
| searchMode={searchMode} | ||
| trace={!!trace} | ||
| searchContextsEnabled={props.searchContextsEnabled} | ||
| patternType={patternType} | ||
| results={results} | ||
| showAggregationPanel={showAggregationPanel} | ||
| selectedSearchContextSpec={props.selectedSearchContextSpec} | ||
| aggregationUIMode={aggregationUIMode} | ||
| caseSensitive={caseSensitive} | ||
| authenticatedUser={authenticatedUser} | ||
| isSourcegraphDotCom={isSourcegraphDotCom} | ||
| enableRepositoryMetadata={enableRepositoryMetadata} | ||
| options={options} | ||
| codeMonitoringEnabled={codeMonitoringEnabled} | ||
| fetchHighlightedFileLineRanges={props.fetchHighlightedFileLineRanges} | ||
| onNavbarQueryChange={setQueryState} | ||
| onSearchSubmit={handleSidebarSearchSubmit} | ||
| onQuerySubmit={handleSearchAggregationBarClick} | ||
| onExpandAllResultsToggle={onExpandAllResultsToggle} | ||
| onSearchAgain={onSearchAgain} | ||
| onDisableSmartSearch={onDisableSmartSearch} | ||
| onLogSearchResultClick={logSearchResultClicked} | ||
| settingsCascade={props.settingsCascade} | ||
| telemetryService={telemetryService} | ||
| platformContext={platformContext} | ||
| extensionsController={extensionsController} | ||
| /> |
There was a problem hiding this comment.
Any chance this could be done without having to create two separate components? (I don't have high hopes but I still want to ask 😆 )
There was a problem hiding this comment.
Hehe, I tried inline changes behind the feature flag, but since this PR has layout and logic changes, I found it very complex to unwrap these behind-feature-flag changes combined with the main logic. Having two components adds some legacy I think but it would be very easy for us to either migrate to a new one or revert to the old version when the time comes.
vovakulikov
force-pushed
the
vk/add-new-search-results-filters
branch
from
e231782
to
13bb4fb
Compare
|
Thanks for the review @fkling
Agreed, at the moment, it should work in some query cases since we open inline-search in the blob UI automatically as we render the blob UI component, it should focus on the first matches and scroll to its match's line, but it doesn't work in all cases; basically, it could be the case when query resolves some matches that are not presented in the query. (basically any OR and AND operator will do this) In theory we could just use matches info from the search blob components and make matches jump in the blob UI based on this info but this will require further improvements in the blob UI (this is something that I wanted to talk about earlier) |
* Extract search result page layout into separate component * Create new search result page layout * Fix styles problem * Connect file preview UI with preview preview button in the results blocks UI * Add active by default inline search for file preview panel * Sync file preview and info bar header heights * Close the panel on query re-submit * Make side blob lazy loaded * Fix ts problem in no results page * Fix ts problems * Fix eslint problems * Update search result info bar snapshots * Add comments to Search Panel View Modes
Closes #57800
Closes #57798
This PR does a few things on the search results page
Further steps
cc @taiyab all changes are behind feature flag, I checked that the current UI hasn't got any regressions from these changes. Hence I don't believe we're blocked by design here. But if you have time and have any suggestions here please share it and I can either make it here or address in follow up PRs.
Test plan
newSearchNavigationUIexperimental feature flag