appbaseio / reactivesearch

Search UI components for React and Vue
https://opensource.appbase.io/reactivesearch
Apache License 2.0
4.89k stars 466 forks source link

Add onFilterCleared prop to SelectedFilters #401

Closed calebdel closed 6 years ago

calebdel commented 6 years ago

Issue Type: Enhancement

Platform: Web

Description: Add a new prop onFilterCleared to SelectedFilters that will be invoked with the value of the selected filter being cleared or it will indicate that all filters have been cleared.

Reactivesearch version: 2.6.2

vivek12345 commented 6 years ago

@metagrover Any example with the code which renders filters?I have figured out how to do this, but I can't seem to figure out from the documentation as to how do I create a list with filters in it.

vivek12345 commented 6 years ago

Actually I figured out how to add filters.

metagrover commented 6 years ago

Sure, we have the components playground as a submodule in this project that you can start with - which runs on the storybook, or alternatively, web examples should come handy.

SelectedFilters docs is a good starting point. Here, we need to add support for onFilterCleared prop as mentioned above.

Thanks for taking out time to look into this 👍

vivek12345 commented 6 years ago

@metagrover should this be called also when someone unchecks the filter or only when I click on the cross icon of the filter to clear it?

metagrover commented 6 years ago

I think the best thing to do here is to only call this function when there is an interaction with the SelectedFilters component, i.e. a filter is cleared. We can call this prop as onClear.

Also, in order to watch for the changes on the SelectedFilters, it'd be best to do that via onChange method, which will be executed anytime there is a change (addition or removal) in selected-values from the redux store.

vivek12345 commented 6 years ago

Alright that's a fair point.I am thinking how will I determine the interaction.Filters can be anything right.A dropdown, a checkbox or anything else.

vivek12345 commented 6 years ago

Okay I think may be setValue can help here because that is called if something is set or removed or may be not.I will think more on this and come back.

metagrover commented 6 years ago

Ideally, you should watch for the changes on the selectedValues from the redux store via componentWillReceiveProps method. But don't worry about that, we don't need to implement that right now - until a valid use-case pops up.

We should only focus on the onClear here for now 🐱

vivek12345 commented 6 years ago

well in that case, this should do the job

remove = (component, value = null) => {
  this.props.setValue(component, null);
  this.props.onClear && this.props.onClear(value);
};

the value here would be

this.renderValue(value, isArray);

This needs some more corrections since now the value is always a string and in case of multi filter removal like removing two filters Rating 3 to 4 and Rating > 4 should return an array of values instead of one single string

metagrover commented 6 years ago

Yes, we should keep things consistent with the values. Also, the function should be called onClear and should return the componentId as well.

So the signature should look like: onClear(componentId, value) where value van be an array or a string.

Still not sure about the clearAll event though. Open to suggestions. @divyanshu013

metagrover commented 6 years ago

Update:

We will only implement onClear(componentId, value) in this case here for now. We can return null for clearAll events.

vivek12345 commented 6 years ago

Alright understood.For the value here should I send it as

[
  {
    "start": 0,
    "end": 3,
    "label": "Rating < 3"
  },
  {
    "start": 3,
    "end": 4,
    "label": "Rating 3 to 4"
  }
]

or with just the label?

metagrover commented 6 years ago

Yes, the entire value object should do the trick since we do some parsing at our end to render the labels.

calebdel commented 6 years ago

@vivek12345 Thanks, this looks great! @metagrover will this be released soon?

metagrover commented 6 years ago

Thanks for the patience on this guys. We will roll it out in this week.