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 classifiers to search #1040

Merged
merged 1 commit into from Apr 18, 2016
Merged

Add classifiers to search #1040

merged 1 commit into from Apr 18, 2016

Conversation

di
Copy link
Member

@di di commented Mar 22, 2016

This PR addresses the "faceting" part of #702 by adding the PyPI classifiers to the search page.

screen shot 2016-03-22 at 5 19 40 pm

A few notes about things I was unsure of:

  • Multiple classifiers act as a union (not an intersection). Not sure if this was the original intention.
  • Selecting a checkbox for a classifier immediately submits the form. I wasn't sure how the UI should work here without introducing a lot of AJAX complexity, and it didn't feel intuitive to have the user scroll back up to the "Search" button to submit the form.
  • A new search from the search bar currently clears the classifiers (and the ordering) thus starting a new search, but this can be easily changed.

Things I am leaving for a future PR (or for @nlhkabu):

@nlhkabu
Copy link
Contributor

nlhkabu commented Mar 22, 2016

This is so exciting! Thankyou @di! Give me a couple of days to pull it down and play with the UI. Ping @dstufft for a code review.

@dstufft
Copy link
Member

dstufft commented Mar 31, 2016

Code wise this looks good to me, so as long as @nlhkabu is happy with the design side it can merge IMO.

@nlhkabu
Copy link
Contributor

nlhkabu commented Apr 17, 2016

ok for me - @dstufft can you please rebase and merge?

@dstufft
Copy link
Member

dstufft commented Apr 17, 2016

@di will need tod o the rebasing unless I make a new PR, which I can do if he'd rather that.

@di
Copy link
Member Author

di commented Apr 17, 2016

I'll rebase tomorrow AM, thanks!

@di di force-pushed the add-classifier-lists branch 2 times, most recently from 69614b2 to a442d8b Compare April 18, 2016 13:43
@di
Copy link
Member Author

di commented Apr 18, 2016

@dstufft Good to go.

@dstufft dstufft merged commit 9e64bc2 into pypi:master Apr 18, 2016
@dstufft
Copy link
Member

dstufft commented Apr 18, 2016

Awesome, thanks!

@di di deleted the add-classifier-lists branch April 18, 2016 14:23
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.

None yet

3 participants