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 Improvements #1609

Merged
merged 16 commits into from
May 1, 2019
Merged

Search Improvements #1609

merged 16 commits into from
May 1, 2019

Conversation

SaraVieira
Copy link
Contributor

@SaraVieira SaraVieira commented Mar 18, 2019

In a user test it was pointed we can't order the search and it makes sense to do so

This PR adds order by likes and views

This PR also:

  • Changes the cards in search for the new cards we have in the explore page
  • Removes how long the search took as that is not important information for the user
  • Changes the API of wide sandbox to have the prop selectSandbox instead of pickSandbox to make it more generic
  • Adds common script
  • Moves some .css files to gobal js styles so we can use the theme

Design
https://www.figma.com/file/akq4c3BMhkW5EHipeIBEbKmf/Search-Page?node-id=0%3A1

@SaraVieira SaraVieira requested a review from CompuIves March 18, 2019 16:00
@SaraVieira SaraVieira changed the title Adds Sort By in Search Search Improvements Mar 21, 2019
@SaraVieira SaraVieira marked this pull request as ready for review March 21, 2019 14:49
@SaraVieira
Copy link
Contributor Author

cc @Haroenv

@Haroenv
Copy link
Contributor

Haroenv commented Mar 21, 2019

The sorting of "relevance" and "views" seems very similar to me, was a different sort required for this?

I'm seeing some flashes when I change index, but I don't see any particular reason why in the code by a quick glance, maybe it already was there before

2019-03-21 15-53-19 2019-03-21 15_54_37

Also, it seems like the search is pretty hidden with the new design, I can't seem to find any link towards it anymore, I had to guess the URL.

Finally there's a small other thing (maybe only on Safari), and that's the search icon isn't correctly positioned:

Screenshot 2019-03-21 at 15 56 36

@SaraVieira
Copy link
Contributor Author

Good points!

I deleted the views one as that is basically the default one and also added a link to the search in the header

The other two:

The flickering seems to only happen on safari :/

I'm looking at the CSS thing now

@CompuIves
Copy link
Member

The bottom part of the card (with description) seems a bit big to me, does it have different styles compared to SandboxCard in common?

@SaraVieira
Copy link
Contributor Author

@CompuIves Nop, itmay be because most of them don't have a description

Copy link
Member

@CompuIves CompuIves left a comment

Choose a reason for hiding this comment

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

This super clean! I left some minor comments. I also think that it would be indeed better to change the background color to what we have on the explore page. Curious what that looks like.

import { Container } from './elements';

function Filters() {
return (
<Container>
<Sort
Copy link
Member

Choose a reason for hiding this comment

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

This is nice! Maybe we can also re-use

image

in a later version.

package.json Outdated Show resolved Hide resolved
@SaraVieira SaraVieira requested a review from CompuIves April 30, 2019 15:24
@CompuIves CompuIves merged commit e8c67ad into master May 1, 2019
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