Skip to content

Conversation

@jeff-phillips-18
Copy link
Member

@jeff-phillips-18 jeff-phillips-18 commented Dec 19, 2017

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

import cx from 'classnames';
import React from 'react';
import PropTypes from 'prop-types';
import { DropdownButton } from 'react-bootstrap';
Copy link
Member

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,
Copy link
Member

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 = {
Copy link
Member

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">
Copy link
Member

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':
Copy link
Member

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, [
Copy link
Member

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">
Copy link
Member

@priley86 priley86 Dec 19, 2017

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...

Copy link
Member

@cdcabrera cdcabrera left a 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>
  );
});

Current...
dec-19-2017 11-29-13

Or something like this with the default template...
dec-19-2017 11-29-30

@jeff-phillips-18
Copy link
Member Author

@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.

@jeff-phillips-18 jeff-phillips-18 changed the title feat(Filter): Add Filter Component [WIP] Add Filter Component Dec 19, 2017
@jeff-phillips-18
Copy link
Member Author

Closing this PR due to significant changes to make filter component(s) stateless

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Component - Filter

4 participants