-
Notifications
You must be signed in to change notification settings - Fork 378
[WIP] Add Filter Component #135
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
Conversation
e2604a5 to
619d68e
Compare
| import cx from 'classnames'; | ||
| import React from 'react'; | ||
| import PropTypes from 'prop-types'; | ||
| import { DropdownButton } from 'react-bootstrap'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can rebase this after #130 merges...
| /* Array of FilterField objects */ | ||
| fields: PropTypes.array.isRequired, | ||
| /* function(field, value) - Callback to call when a filter is added */ | ||
| filterAddedCB: PropTypes.func, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we camel case these names, i.e. filterAddedCallback or onFilterAdded?
| return; | ||
| } | ||
|
|
||
| var updateState = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
var becomes let
| </div> | ||
| <div className="container-fluid"> | ||
| <div className="row"> | ||
| <div className="col-xs-12"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can use the Col and Row components now..
| if (value !== undefined) { | ||
| if (currentField.filterType === 'complex-select') { | ||
| switch (valueType) { | ||
| case 'filter-category': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we pull these literals in the case statements out into a constants.js file.? I think it would line up more with the Charts/ToastNotifications etc..
| filterCategory: null, | ||
| filterValue: null, | ||
| }; | ||
| bindMethods(this, [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!
| <hr /> | ||
| </div> | ||
| <div className="col-xs-12"> | ||
| <label className="events-label pull-left"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it will be nice when #131 is merged... then we can use ControlLabel and the other Form elements (i'm also guilty of this 😸 ). Maybe we can rebase all Form examples after it's in...
cdcabrera
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What type of Storybook entry are we going for? If it's the second of the two animated Gifs, the code for the story can be trimmed down to (with some additional mods too)...
stories.addDecorator(
defaultTemplate({
title: 'Filter',
documentationLink:
'http://www.patternfly.org/pattern-library/forms-and-controls/filter/',
}),
);
stories.addWithInfo('Filter', () => {
return (
<Row>
<Col sm={12}>
<MockFilterDisplay />
</Col>
</Row>
);
});|
@cdcabrera That second version you show is not really helpful to anyone looking for filter component information. It only speaks to the components used to display the example. I thought it better to spell out the parameters and the structure of the filter field objects. |
|
Closing this PR due to significant changes to make filter component(s) stateless |


What:
Adding basic Filter component
Link to Storybook:
https://jeff-phillips-18.github.io/patternfly-react/?selectedKind=Filter&selectedStory=Filter
closes #106
Waiting on styling to go into Patternfly core: patternfly/patternfly#894
FYI @serenamarie125