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.

Sign up for GitHub

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

[NEP-12629] Ability to use "OR" between filters (+ filter groups) #108

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from
Open

[NEP-12629] Ability to use "OR" between filters (+ filter groups) #108

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
e6ad99e
[NEP-12629] Rework filters logic by adding AND/OR conditions.
Jan 17, 2022
2a41da1
[NEP-12629] Bump version to 2.1
Jan 24, 2022
affdf0e
[NEP-12629] Fix "X" icon position
Jan 24, 2022
95206e3
[NEP-12629] Dist
Jan 27, 2022
d629f0f
[NEP-12629] Update example on server
Feb 1, 2022
0ad2738
[NEP-12629] Initial URL filters
Feb 1, 2022
df27902
[NEP-12629] Change version to 3.0
Feb 8, 2022
828327a
[NEP-12629] Fix filters default value.
Feb 14, 2022
d52e253
[NEP-12629] Fix default filters URLs with old format for SavedSearches.
Mar 1, 2022
a4b3b44
[NEP-12629] Fix filter labels for SavedSearch
Mar 14, 2022
b012d4f
[NEP-12629] Add debug version to console.log
Mar 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bower.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "react-filterbar",
"homepage": "https://github.com/jobready/react-filterbar",
"version": "1.7.1",
"version": "2.1",
Copy link
Member

Choose a reason for hiding this comment

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

Odd that this was out of sync with the package file.

Copy link
Member

Choose a reason for hiding this comment

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

Would this be better represented by a major upgrade 3.0 since there may not exist backward compatibility with previous design?

"authors": [
"Jacob Bass <[email protected]>"
],
Expand Down
530 changes: 430 additions & 100 deletions dist/react-filterbar.js
View file
Open in desktop

Large diffs are not rendered by default.

27 changes: 14 additions & 13 deletions dist/react-filterbar.min.js
View file
Open in desktop

Large diffs are not rendered by default.

29 changes: 29 additions & 0 deletions docs/react-components-structure.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
React Components Structure
Copy link
Member

Choose a reason for hiding this comment

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

Good to see this here ✨

--------------------------

FilterBar
-> FilterList
-> FilterListOption
-> ApplyFiltersButton
-> ClearFiltersButton
-> SaveFiltersButton
-> SavedSearchesList
-> ConfigurationButton *
-> ExportResultsButton *
-> BatchActionsList
-> FilterDisplay
-> FilterGroup
-> FilterItem
-> FilterButton
-> FilterButton


{} - available filters
[] - default filters
[
[filter1] -> when adding new add new filter1
]

[
[filter1, filter2] -> when adding "ADD" button and add new filter2
]
533 changes: 432 additions & 101 deletions example/public/js/react-filterbar.js
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "react-filterbar",
"version": "2.0",
"version": "2.1",
Copy link
Member

Choose a reason for hiding this comment

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

As mentioned above, major upgrade perhaps?

Copy link
Author

Choose a reason for hiding this comment

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

@msxavi I'm happy with any version actually and if you think 3.0 better reflects current changes and improvements we can change it 👌

"description": "",
"main": "dist/react-filterbar.js",
"engines": {
Expand Down
12 changes: 6 additions & 6 deletions src/actors/FilterBarActor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,6 @@ export class FilterBarActor {
this.filterBarStore.enableFilter(filterUid, value);
}

disableFilter(filterUid) {
this.filterBarStore.disableFilter(filterUid);
}

disableAllFilters() {
this.filterBarStore.disableAllFilters();
this.filterBarStore.disableAllQuickFilters();
Expand All @@ -37,8 +33,12 @@ export class FilterBarActor {
this.applyFilters();
}

updateFilter(filterUid, propKey, propValue) {
this.filterBarStore.updateFilter(filterUid, propKey, propValue);
updateFilter(groupKey, inputKey, value) {
this.filterBarStore.updateFilter(groupKey, inputKey, value);
}

disableFilter(groupKey, inputKey) {
this.filterBarStore.disableFilter(groupKey, inputKey)
}

applyFilters() {
Expand Down
4 changes: 0 additions & 4 deletions src/components/FilterBar/FilterBar.react.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,6 @@ export class FilterBar extends React.Component {
<div>
<div>
<div className="btn-group margin-bottom-sm">
<FilterList
disabledFilters={this.context.filterBarStore.getDisabled()}
/>
Comment on lines -21 to -23
Copy link
Member

Choose a reason for hiding this comment

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

What was the purpose of this previously?

Copy link
Author

@footnoise footnoise Jan 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah @SyntheticDave sorry, potentially I should put that architectural changes in PR description, because it's a huge changes. Definitely will do it before moving PR into Release stage.

So, before we had registered filters in a hash and it was represented in FilterBarStore like that:

{
  "some_filter_key1": {
    "uid" : "some_filter_key1",
    "value" : "",
    "name" : "Some Filter Key 1",
    ...
  }, {
    "some_filter_key2" : {
    "uid" : "some_filter_key2",
    "value" : "",
    "name" : "Some Filter Key 2",
    ...
  }
}

and if a User selected some filters, for example with the key some_filter_key1 we needed mark it somehow and don't allow user to select that filter again (it should be removed from dropdown) and to make it we should call method this.context.filterBarStore.getDisabled() and pass it as disabledFilters property into component.

The new logic works in completely different way, we still have original registered hash of filters (as I described above) but data and values we have in the same FilterBarStore but in activeFilters array. In this case I use groupKey and inputKey properties to manage values data. It's a double array, which has groupKey as ID of the first array and inputKey as ID for the second array.

So, that means we should hide filters from dropdown and we don't need to set property disabledFilters anymore.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the explanation. I definitely like the change, but you're right that it's a big one.


<ApplyFiltersButton
filterBarActor={this.context.filterBarActor}
/>
Expand Down
63 changes: 63 additions & 0 deletions src/components/FilterBar/FilterDisplay/FilterButton.react.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import { FilterListOption } from "../FilterList/FilterListOption.react";
export class FilterButton extends React.Component {
constructor(props) {
super(props);

this.state = { filters: props.filters };
}

componentDidMount() {
this.context.filterBarStore.addChangeListener(this.onChange.bind(this));
}

onChange() {
this.setState(this.getStateFromStores());
}

getStateFromStores() {
return {
filters: this.context.filterBarStore.getFilters()
};
}

onClick(filterUid) {
this.props.onClick(filterUid);
}

render() {
var optionKey = "";
var filterOptions = Object.keys(this.state.filters).map(function(filterUid) {
optionKey = "option-" + filterUid;
return (
<FilterListOption
onClick={ this.onClick.bind(this) }
filterUid={filterUid}
key={optionKey}
label={this.state.filters[filterUid].label}
/>
);
}, this);
return (
<div className='btn-group'>
<button className='btn btn-default dropdown-toggle' data-toggle='dropdown' type='button'>
<span>{ this.props.title }</span>
<i className='icon icon-add'></i>
</button>
<div className='dropdown-menu' role='menu'>
<ul className='filter-options'>
{filterOptions}
</ul>
</div>
</div>
)
}
}

FilterButton.contextTypes = {
filterBarActor: React.PropTypes.object,
filterBarStore: React.PropTypes.object
};

FilterButton.propTypes = {
disabledFilters: React.PropTypes.object.isRequired
};
72 changes: 56 additions & 16 deletions src/components/FilterBar/FilterDisplay/FilterDisplay.react.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import {FilterInput} from "./FilterInput.react";

import {FilterButton} from "./FilterButton.react";
import {FilterGroup} from "./FilterGroup.react";
import { FilterList } from "../FilterList/FilterList.react";
export class FilterDisplay extends React.Component {
constructor(props) {
super(props);
Expand Down Expand Up @@ -40,30 +42,68 @@ export class FilterDisplay extends React.Component {
};
}

getActiveFilters() {
return this.context.filterBarStore.getActiveFilters();
}

getFilters() {
return this.context.filterBarStore.getFilters();
}

addGroup(filterUid) {
this.context.filterBarStore.addGroupFilter(-1, filterUid);
Copy link
Member

Choose a reason for hiding this comment

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

Not a big deal, but if you updated addGroupFilter to accept the groupKey last, you could just leave it off the argument list here, and just check for groupKey == undefined rather than groupKey > 0.

Copy link
Author

Choose a reason for hiding this comment

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

Good thinking, will do that!

}

render() {
var filters = Object.keys(this.state.filters).map(function(filterUid) {
var filter = this.state.filters[filterUid];
var filters = [];
this.getActiveFilters().map(function(groupFilters, idx) {
if (idx > 0) {
filters.push(
(
<div style={ { marginTop: 'auto', marginBottom: 'auto', padding: '10px'} }>OR</div>
)
)
}

return (
<FilterInput
filterUid={filterUid}
key={filterUid}
label={filter.label}
type={filter.type}
value={filter.value}
operator={filter.operator}
/>
filters.push(
(<FilterGroup
key={ idx }
groupKey={ idx }
filters={ groupFilters }
/>)
Copy link
Member

Choose a reason for hiding this comment

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

So does this mean if there's no group specified, all filters get automatically added to a new first group?
If so, it seems like existing saved searches and existing links to filters will still work correctly? Would be amazing if so.

Copy link
Author

Choose a reason for hiding this comment

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

Well, yeah, that's a good point about SavedSearches. My thinking was that SavedSearches should be presented as is with OLD format (w/o groups) and should be converted to NEW format (w/ groups) when new react component loads it the first time and SavedSearch should be saved in NEW format after if user decides to save it.

Copy link
Member

Choose a reason for hiding this comment

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

We'll definitely want to make sure we don't break existing saved searches or links in the application that direct to filtered index pages (e.g. reports often link to an index page with the same filters as is used by that cell in the report), so we'll need to make sure this is backwards compatible with those.

);
}, this);

})

if (filters.length === 0) {
filters = (<div>No Filters Enabled!</div>);
filters.push((
<div style={ { marginTop: 'auto', marginBottom: 'auto', padding: '10px'} }>
Copy link
Member

Choose a reason for hiding this comment

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

There's a bunch of inline styling here. Is it instead worth creating classes for this so they can be overridden at the application level if needed?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, currently thinking how to make it better and replace inline styles with reusable css.

Copy link
Member

Choose a reason for hiding this comment

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

Don't stress too much about it. If it's too big a job to do so, I'd create a tech debt ticket to add that in later, as inline styles should be okay in the short term.

<FilterButton
filters={ this.getFilters() }
title="ADD FILTER"
onClick={ this.addGroup.bind(this) }
/>
</div>)
);
} else {
filters.push(
(
<div style={ { marginTop: 'auto', marginBottom: 'auto', padding: '10px'} }>
<FilterButton
filters={ this.getFilters() }
title="OR"
onClick={ this.addGroup.bind(this) }
/>
</div>
));
}

return (
<div className="navbar filterbar">
<div className="panel panel-default">
{filters}
<div className="panel panel-default" style={ { paddingTop: 'unset', paddingBottom: 'unset' } }>
<div style={ { display: 'flex', float: 'left', flexWrap: 'wrap' } }>
{filters}
</div>
</div>
</div>
);
Expand Down
71 changes: 71 additions & 0 deletions src/components/FilterBar/FilterDisplay/FilterGroup.react.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import { FilterInput } from "./FilterInput.react"
import { FilterButton } from "./FilterButton.react"

export class FilterGroup extends React.Component {
constructor(props) {
super(props);

this.state = { filters: props.filters };
}

getFilters() {
return this.context.filterBarStore.getFilters();
}

onButtonClick(filterUid) {
this.context.filterBarStore.addGroupFilter(this.props.groupKey, filterUid);
}

render() {
const { groupKey } = this.props;
var filters = [];
this.state.filters.map(function(filter, idx) {
if (idx > 0) {
filters.push(
(
<div style={ { marginTop: 'auto', marginBottom: 'auto', padding: '10px'} }>AND</div>
)
);
}

filters.push(
(
<div style={ { marginTop: 'auto', marginBottom: 'auto', padding: '10px'} }>
<FilterInput
groupKey={ groupKey }
inputKey={ idx }
filterUid={filter.uid}
key={filter.uid}
label={filter.label}
type={filter.type}
value={filter.value}
operator={filter.operator}
/>
</div>)
);
});

filters.push(
(
<div style={ { marginTop: 'auto', marginBottom: 'auto', padding: '10px'} }>
<FilterButton
filters={ this.getFilters() }
title="ADD"
onClick={ this.onButtonClick.bind(this) }
/>
</div>
)
);

return (
<div style={ { display: 'flex', flexWrap: 'wrap', borderRadius: '5px', border: '1px solid #c0c0c0', backgroundColor: '#eee', marginTop: '7px', marginBottom: '7px' } }>
{filters}
</div>
)
}
}

FilterGroup.contextTypes = {
filterBarActor: React.PropTypes.object,
filterBarStore: React.PropTypes.object
};
9 changes: 6 additions & 3 deletions src/components/FilterBar/FilterDisplay/FilterInput.react.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,17 @@ export class FilterInput extends React.Component {
}

onClick() {
this.context.filterBarActor.disableFilter(this.props.filterUid);
const { groupKey, inputKey } = this.props;
this.context.filterBarActor.disableFilter(groupKey, inputKey);
Copy link
Member

Choose a reason for hiding this comment

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

Does this mean I can't add a filter for the same property into the one group, but can still use it in other groups?

Copy link
Author

Choose a reason for hiding this comment

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

Well, I think it's a bad naming here. New logic allows you to use filter as many times as you want in one or many groups. Potentially method should be called removeFilterValue or unsetFilterValue & etc because in current configuration is nothing to disable.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, right. Yeah, I think remove or unset would be a more descriptive method name then.

}

objectProperties() {
var key = Date.now();
return(
{
filterUid: this.props.filterUid,
groupKey: this.props.groupKey,
inputKey: this.props.inputKey,
key: key,
value: this.props.value,
type: this.props.type,
Expand All @@ -26,11 +29,11 @@ export class FilterInput extends React.Component {
var propObject = this.objectProperties();
var inputs = new FilterInputFactory(propObject);
return (
<div className="col-lg-3 col-md-4 col-sm-6 col-xs-12 filter">
<div className="filter">
<ul className={this.filterKey}>
<li>
<i
className="btn btn-circle-primary btn-xs icon icon-close remove-filter"
className="btn btn-circle-primary btn-xs icon icon-close remove-filter" style={ { lineHeight: '16px' } }
Copy link
Member

Choose a reason for hiding this comment

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

As with elsewhere, I'd really like to see this done with css classes to maintain flexibility.

Copy link
Author

Choose a reason for hiding this comment

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

Yep, working on it! 👍

onClick={this.onClick.bind(this)}
/>
<label>
Expand Down
2 changes: 1 addition & 1 deletion src/components/FilterBar/FilterDisplay/Inputs/DateInput.react.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export class DateInput extends React.Component {
}

onBlur() {
this.context.filterBarActor.updateFilter(this.props.filterUid, "value", this.state.value);
this.context.filterBarActor.updateFilter(this.props.groupKey, this.props.inputKey, this.state.value);
Copy link
Member

Choose a reason for hiding this comment

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

So inputKey used to just be "value"?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, groupKey and inputKey here are just IDs where exactly values should be set. Object filterBarActor has activeFIlters array with filters values which can be presented like that:

activeFIlters = [
   ["value1", "value2"],
   ["value3", "value4"]
]

and if we need to update value for example for filter2 ("value2" is located in 1st group) we should pass groupKey as 0 (first index in array) and inputKey as 1 (second index in array)

}

componentDidMount() {
Expand Down
2 changes: 1 addition & 1 deletion src/components/FilterBar/FilterDisplay/Inputs/DateTimeInput.react.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export class DateTimeInput extends React.Component {
}

onBlur() {
this.context.filterBarActor.updateFilter(this.props.filterUid, "value", this.state.value);
this.context.filterBarActor.updateFilter(this.props.groupKey, this.props.inputKey, this.state.value);
}

componentDidMount() {
Expand Down
1 change: 1 addition & 0 deletions src/components/FilterBar/FilterDisplay/Inputs/LazyMultiSelectInput.react.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export class LazyMultiSelectInput extends React.Component {
} else {
filter.value = event.target.value.split(",");
}
this.context.filterBarActor.updateFilter(this.props.groupKey, this.props.inputKey, filter.value);
Copy link
Member

Choose a reason for hiding this comment

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

Do you know why this was not needed for this filter type previously?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, because react-component and filters had different architecture & structure before (see my first comment above) but now we need to set selected data to activeFilters array.

}

render() {
Expand Down
1 change: 1 addition & 0 deletions src/components/FilterBar/FilterDisplay/Inputs/LazySelectInput.react.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export class LazySelectInput extends React.Component {
onSelect(event) {
let filter = this.context.filterBarStore.getFilter(this.props.filterUid);
filter.value = event.target.value;
this.context.filterBarActor.updateFilter(this.props.groupKey, this.props.inputKey, filter.value);
}

render() {
Expand Down
Loading