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

Reorganize default search filters #1275

Merged
merged 7 commits into from
Mar 25, 2020
Merged

Conversation

kpuputti
Copy link
Contributor

@kpuputti kpuputti commented Mar 20, 2020

This PR reorganizes the default search filters in the search page. The category and amenities filters are moved from primary filters to secondary filters under the "More filters" panel.

This is mainly for making more space to the UI in preparation for the sorting controls.

Before

Screenshot 2020-03-20 at 10 35 55

After

Screenshot 2020-03-20 at 10 36 12

Screenshot 2020-03-20 at 10 36 25

@kpuputti kpuputti marked this pull request as ready for review March 20, 2020 08:42
@@ -101,7 +104,7 @@ class SearchFiltersPanelComponent extends Component {
// if no option is passed, clear the selection for the filter
const currentQueryParams = option
? { ...mergedQueryParams, [urlParam]: option }
: omit(mergedQueryParams, urlParam);
: { ...mergedQueryParams, [urlParam]: null };
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't remove omit. (I assume that this will add URL parameter category=null&amenities=null)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, this was a bug fix. Clearing the filter didn't work without this. It removes the param from the URL correctly.

@@ -43,7 +43,7 @@ class MainPanel extends Component {

const isSearchFiltersPanelOpen = !!secondaryFilters && this.state.isSearchFiltersPanelOpen;

const filters = merge(primaryFilters, secondaryFilters);
const filters = merge({}, primaryFilters, secondaryFilters);
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@Gnito Gnito left a comment

Choose a reason for hiding this comment

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

Only one change request, but I'm wondering if we should postpone the merging of this PR to the point when this change makes sense (i.e. when there's more dropdown coming to fill the space)

@kpuputti kpuputti mentioned this pull request Mar 23, 2020
@kpuputti kpuputti merged commit 2ea7ffa into master Mar 25, 2020
@kpuputti kpuputti deleted the reorganize-default-search-filters branch March 25, 2020 14:51
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.

2 participants