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

Add sidebar with GUI filters to the search results page #103

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

axispx
Copy link

@axispx axispx commented Sep 29, 2019

This PR adds interactive filters in the sidebar to filter between repositories and languages.

filters

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@axispx
Copy link
Author

axispx commented Sep 29, 2019

@googlebot I signed it!

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@hanwen
Copy link
Contributor

hanwen commented Sep 29, 2019

nice!

I usually do the reviews from Gerrit though - see CONTRIBUTING. Would you mind sending me a change there instead?

@axispx axispx force-pushed the master branch 3 times, most recently from dc647d5 to 04c3621 Compare October 2, 2019 05:42
@axispx
Copy link
Author

axispx commented Oct 2, 2019

There's an error adding my GitHub email address so I'm unable to push the changes to Gerrit.

@hanwen
Copy link
Contributor

hanwen commented Oct 16, 2019

I had a quick look at this last week. I have two complaints:

  • the left nav doesnt scale down when you resize the window
  • overall, the left pane takes up valuable real estate if you scroll down.

I think this feature would make more sense as a dropdown menu.

@axispx
Copy link
Author

axispx commented Nov 13, 2019

@hanwen noted. I'll put in some changes soon.

@hanwen
Copy link
Contributor

hanwen commented Jan 27, 2020

ping?

@axispx
Copy link
Author

axispx commented Jan 28, 2020

Hi, I got busy with work and completely forgot about this. I'll work on this tonight. Thanks for the reminder.

@hanwen
Copy link
Contributor

hanwen commented Feb 24, 2020

sorry for the delay, i have been busy with other things

(sending through Gerrit as described in CONTRIBUTING usually yields faster feedback).

  • if you click on a result in the -print version (the local viewer), you get the restrictions too, but they refer to the single file, and it doesn't make sense. I think the buttons should be disabled or simply not be there.

  • the list of choices (repos, languages) looks alphabetical. I think ti should be ordered by frequency

  • If there are no results, the dropdown becomes empty. Maybe remove or disable the button in those cases?

  • if you edit the query, the dropdown potentially is invalid. Maybe not a big issue. Thoughts?

@hanwen
Copy link
Contributor

hanwen commented Feb 24, 2020

implementation issue: you look at substrings ("lang:XXX") in the backend to populate the checkboxes.

This is incorrect if those bits appear as substrings eg.

clang:c

In the server, you can have access to the parsed query, so you should instead find the toplevel query.And and then look at its children to find all query.Repo/query.Language atoms.

@hanwen
Copy link
Contributor

hanwen commented Feb 25, 2020

it also needs some work so the repo list view doesn't get messed up.

The drop down shouldn't be there for the repolist.

--- FAIL: TestBasic (0.00s)
e2e_test.go:230: query "/search?q=r:": result did not have "1234">master": template: navbar:33:37: executing "navbar" at <.FileMatches>: can't evaluate field FileMatches in type *web.RepoListInput
e2e_test.go:230: query "/search?q=r:": result did not have "Found 1 repositories": template: navbar:33:37: executing "navbar" at <.FileMatches>: can't evaluate field FileMatches in type *web.RepoListInput
e2e_test.go:230: query "/search?q=r:": result did not have "Feb 25, 2020 09:29": template: navbar:33:37: executing "navbar" at <.FileMatches>: can't evaluate field FileMatches in type *web.RepoListInput
e2e_test.go:230: query "/search?q=r:": result did not have "repo-url">name": template: navbar:33:37: executing "navbar" at <.FileMatches>: can't evaluate field FileMatches in type *web.RepoListInput
e2e_test.go:230: query "/search?q=r:": result did not have "1 files (36)": template: navbar:33:37: executing "navbar" at <.FileMatches>: can't evaluate field FileMatches in type *web.RepoListInput

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

Successfully merging this pull request may close these issues.

3 participants