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

Reset filter #2186

Closed
ivanmalagon opened this issue Aug 27, 2018 · 3 comments
Closed

Reset filter #2186

ivanmalagon opened this issue Aug 27, 2018 · 3 comments
Assignees

Comments

@ivanmalagon
Copy link
Contributor

ivanmalagon commented Aug 27, 2018

Context

While developing a dashboard using Airship I run into some workflows that can be improved (related to filters).

Reset filter

If I want to reset a filter, that is, an existing filter not to be applied, I need now to remove the filter. Then, when I need the filter to be applied again, I have to create a new filter and add it to the source.

Think on using a filter with a histogram widget, for instance.

It'd be better if the filter had a state of 'no filter' so it's added to a source but it don't produce any effect to the SQL. So the user should only worry about setting its value, instead of creation / remove.

Combining filters

With the other point in mind, it needs to be possible to have two filters at the same time, one with no value.

Context: when setting a Range filter and a category filter in the Airship demos dashboard, if a range filter with no values set yet was combined with a particular range in category widget, the produced SQL had an error. It was like

... WHERE (AND neighbourhood IN 'Retiro')

Since the filter was attached but it was not set, the filter SQL had this AND but lacking one of the sides.

Take that into account when developing the reset filter.

Extra ball

Can you add the subselect filter in the category filter? We've already a customer asking for it and it was one of the things you did in a first iteration (but I dropped it because I thought it'd overkilling 😞 my bad)

@jesusbotella
Copy link
Contributor

I totally agree with the filters combination bug, but I do not agree with the reset filters thing completely though 😂

The thing is that the filters are meant to be reused all over the application, and you don't need to create a new filter if you had one with the same purpose already.

So the ideal workflow would be:

const districtFilter = new carto.filter.Category('district_group', { in: ['Diamond District', 'Tudor City', 'Little Brazil'] });
source.addFilter(districtFilter);

If you don't need the filter anymore, you can remove it but it will be still available if you need it later.
So that you can do source.addFilter(districtFilter) in other part of the code.

And if any developer doesn't like that approach, they can do what you suggested when we fix the empty filters combination (It is not ideal, and I think it is harder to understand though).

const fakeFilter = new carto.filter.Category('fakeColumn', {});
fakeFilter.setFilters({})

@ivanmalagon
Copy link
Contributor Author

What I mean is that no filter should be a valid filter state for a given filter. So you don't have to deal with sources and filters but only with the particular filter. At least, that's what I felt working with them.
I like that setFilters({})

I didn't think on reusing the instance, though.

@jesusbotella
Copy link
Contributor

Deployed! 🚀

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

No branches or pull requests

2 participants