RFC14 Autocomplete Suggestions - Front-End
Created by: ghost
RFC 14: Improve autocomplete suggestions
Changes:
- Updated suggestions mechanism of app search,
<QueryInput>
, to use new suggestions format - Some suggestions now static and some dynamic
Selecting a repo no longer goes to URL, only adds suggestion to input (need to review this)
TODO:
-
Tests / Fix minor bugs -
@unknwon can we create a [standard format][baseSuggestions] for suggestions? -
Implement fuzzy-search together with filters selection (see below, update 29/Oct/19) -
Review which suggestions are shown
I'll be trying to fix the overflow to implement the keyword highlighting. Please give any feedback on this initial implementation PR. I took a while to get going but now everything is working well locally and I've found my way around the code.
This implementation should help align back-end x front-end, but as we have more time till 3.10 maybe we can discuss refactoring or any improvements to the search component?
Updates:
-
21/Oct/19: Keyword highlighting to be implemented in separate PR
-
29/Oct/2019: @lguychard @christinaforney and @hadrian-git reviewed the current implementation and decided on joining it with the current fuzzy-search implementation. @hadrian-git will work on using the current fuzzy-search API with new filter suggestions UX and receive feedback afterwards.
- When a new word is being typed, show suggestions from fuzzy-search and filter types, with filters shown on top.
- If a filter type is selected and it has static values, show static suggestions
- If a filter type is selected and it has dynamic values, fetch suggestions from back-end
- Try to use current fuzzy-search API to fetch these dynamic values
- If filters were selected in the query, and a word is typed, do a scoped fuzzy-search
-
12/Nov/2019: @christinaforney, @unknwon and @hadrian-git discussed on keyword highlighting and decided to have developed only the “invalid token” highlighting (described in the RFC as “squiggly underline”). The other feature of “boxing” filters/values will be revisited after the “invalid token” highlighting is done.
-
19/Nov/2019: Fixes for release 3.10: #6662 #6685
Merge request reports
Activity
Created by: ghost
What's in the GIF looks awesome!
[ ] @unknwon can we create a [standard format][baseSuggestions] for suggestions?
Could you given an example of what "[standard format][baseSuggestions]" you're referring?
Ah, link was broken. I meant this section
It's not a big deal as they can be formatted on front-end during page load, but just checking if it's viable for back-end to have all suggestions follow a standard format:
type Suggestion = { value: string description: string | undefined } type FilterSuggestions = { [filterName: string]: { default: string | undefined values: Array<Suggestion> } }
Created by: unknwon
@hadrian-git:
It's not a big deal as they can be formatted on front-end during page load, but just checking if it's viable for back-end to have all suggestions follow a standard format:
type Suggestion = { value: string description: string | undefined } type FilterSuggestions = { [filterName: string]: { default: string | undefined values: Array<Suggestion> } }
Assuming we're both looking at the "default" value. I think the backend (i.e. GraphQL API) should not return it, at least not now. Because currently only "repo" and "repogroup" are returned and they don't have "default" values (no use case of this in terms of GraphQL API). Plus only filters with enum values will have the default value, which are now all being encoded in frontend. WDYT?
Created by: ghost
Added a first round of comments on the code below.
Selecting a repo no longer goes to URL, only adds suggestion to input (need to review this)
Was this part of RFC14?
@lguychard The removal of the URL redirect wasn't explicitly defined on RFC14, but it shows having the
repo
filter inserted together with other filters without the URL redirect, so apparently the URL redirect should be disabled?Created by: felixfbecker
Idea on selecting
repo
filters: could we make it so that when you submit a query that only has one exact repo filter (i.e. starts with^
and ends with$
with no special regex characters in between) it goes straight to the repo? It would only give you one search result anyway (that one repo) so I think that would be a good shortcut. It would basically mean you would just have to double-tap ⏎ to go to a repo suggestion.We had solved this in the past by making ⏎ and tab do different things but that was confusing to users, so I think this is better.
Created by: unknwon
@hadrian-git:
Yeh, the filters that don't have default values don't need to provide any, as the type in the front-end is defined with
undefined
too. Was asking more as an opportunity to simplify and have consistency on how suggestions are defined.I think how
FilterSuggestions
is defined right now looks good to me.Created by: ghost
Idea on selecting
repo
filters: could we make it so that when you submit a query that only has one exact repo filter (i.e. starts with^
and ends with$
with no special regex characters in between) it goes straight to the repo? It would only give you one search result anyway (that one repo) so I think that would be a good shortcut. It would basically mean you would just have to double-tap ⏎ to go to a repo suggestion.We had solved this in the past by making ⏎ and tab do different things but that was confusing to users, so I think this is better.
@felixfbecker so searches starting with
repo:
(or any other filter) would not redirect, and searches starting with^
would show repo values for autocomplete and redirect after selection or ⏎?Created by: felixfbecker
@hadrian-git not sure if I understand your question - only searches that look exactly like this would redirect (example for sourcegraph/sourcegraph):
repo:^github.com/sourcegraph/sourcegraph$
. Everything else would show results as usual.Although we may want to also redirect exact file queries like
repo:^github.com/sourcegraph/sourcegraph$ file:^README.md$
, since currently we also jump to those directly, not just for repos.Created by: ghost
@felixfbecker I was trying to clarify on how the query would look like, but your reply is clear. When the only those filters (
repo
andfile
) are selected and a search is done then there should be a redirect to the exact URL (instead of search results, which will be the case when there are other filters)Created by: ghost
@sourcegraph/web I implemented unit tests for the new helper functions and will leave the e2e for the next PR where I'll implement keyword highlighting. I'm planning on implementing the URL redirect (and
eventLogger.log
on methodQueryInput.onSuggestionSelect
) in the next PR too (or a separate PR), wdyt?Created by: ghost
any recommendations for the icons? !6105 (merged)R39
and anything else I should work on for this PR? (I've branched and started working on keyword highlighting)
Created by: ghost
Looks good overall!
* See one comment about using `mdi-react` icons. * I'm still seeing this behaviour:
If I select a filter, then type space, or delete the entire query, I get the dropdown with all suggestions. Is this expected? My impression was that we only wanted to show suggestions that matched a filter that was being typed, so maybe this is a bug in filterSearchSuggestions()?
Is it expected?
@lguychard I saw that happening after fixing the last bug. The RFC does not describe listing all filters, only filters for at least a single character. Personally I found it helpful seeing all possible filters right at the search box, but of course this behaviour can easily be prevented. wdyt?
Created by: christinaforney
I AM SO EXCITED ABOUT THIS!!
I literally ran around the office showing people Great work!A few things I found:
-
When specifying a language first, then trying to select a repo, I am getting only file results. I would expect to still get repositories listed here.
-
Similarly, when suggestions are given that are not the same as the given filter, it feels like the right solution would be to add the correct filter (though this might be a little strange). I would like to discuss further, but my current strawman suggestion would be:
- If a suggestion is clicked and it uses a different filter, we should update the filter to match. Here's an example:
- In this case, I have specified and started typing a
repo
filter, but then choose a file that is inside of a specific repo. I would expect to see two things if I chose that option:repo:sourcegraph-rewrite-in-x86-asm
andfile:README.md
- Ultimately, the case I want to avoid is what happens currently: when selecting README.md, I get the following added to my search query, which returns an error:
repo:README.md
- If a suggestion is clicked and it uses a different filter, we should update the filter to match. Here's an example:
-
From @ryan-blunden: what is the keyboard shortcut to show options on a blank input? I figured out I could press spacebar to show the list of filters, but it would be nice to have a shortcut to show those results (E.g. ctrl-spacebar?)
-
On no results, the suggestions aren't shown on your branch (right) but are shown on sourcegraph.com (left), but they do show up when results are found. Was this an intentional decision? Ultimately these suggested filters won't be shown in this "mode" but I wasn't expecting that to change until we started implementation of the two different modes.
-
Filters shown in the list:
-
-repohasfile
isn't listedin the options, but is a valid filter in our docs - More complex case (non-blocking): There are a few filters that are valid only if a
type:diff
ortype:commit
are already in the query:author
,before
,after
,message
. It would be nice to show these options only if the query is of type diff/commit.
-
Generally, I would bias towards getting this on sourcegraph.com (merged to master) as quickly as possible, so we can get real feedback from the team as they use it in day-to-day work flows. This way we still have a bit more time before the release to make additional improvements.
-
Created by: ghost
@christinaforney
1'. Nice catch, I forgot to filter the results for
repo
, I'll send an update in a bit.3'. I'll add ctrl+space, currently typing
:
separatly shows all filters.4'. Was not intentional, I'll check on why it's happening.
5.i. I'll also add on next commit. 5.ii. Could we add this in the next PR? (It'll need new code to check on what's already in query)
Created by: christinaforney
@christinaforney the last commits should improve on points 1. and 2, wdyt? (I'll continue working on the others)
Looks good! 1. is great. For 2. it looks like you just restricted the results to only be of the type the filter allows (e.g files or repos for file/repo respectively) - is that correct?
One thing I noticed is that the
file
filter now only shows the set filter suggestions, but not the actual file results once starting to drill down (e.g. in a repo with a README, and I typefile:REA
I'm not seeing that as a suggestion. Is that expected?