Skip to content

Conversation

@dtaylor113
Copy link
Member

@dtaylor113 dtaylor113 commented Aug 27, 2018

What:
Updated the filter component to latest designs outlined here:

  1. Removed placeholder text within the dropdowns
  2. Placeholder are italicized, selected values are not
  3. Restricted Select/Dropdown filters to single select/filter application
  4. Dropdowns remember previous selection when switching between filter fields.
  5. Dropdowns revert to placeholder when it's filter is removed.
  6. Fixed filter unit tests
  7. Removed eslint warnings from _compound-label.scss

@mcarrano

Related PR: patternfly/patternfly-ng#449

Link to Storybook:
Storybook: Filter

  • Here is a screen recording:
    react-filter

Additional issues:
Fixes #553

@dtaylor113
Copy link
Member Author

I'm debating whether this should be a fix or a feat. For patternfly-ng it was a fix, however the changes to complex-select make this PR more complex, so erring on the side of feat

@coveralls
Copy link

coveralls commented Aug 27, 2018

Pull Request Test Coverage Report for Build 2012

  • 3 of 3 (100.0%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 78.472%

Totals Coverage Status
Change from base Build 2002: 0.02%
Covered Lines: 2854
Relevant Lines: 3389

💛 - Coveralls

.filter-selected {
font-style: normal;
color: initial;
}
Copy link
Member

Choose a reason for hiding this comment

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

This belongs in patternfly core and should likely be .filter-pf-select-dropdown.filter-selected. The patternfly test pages show filters with selected categories and won't match patternfly-react if we only do this here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, sounds good. What test pages though? I can't select anything here: https://rawgit.com/patternfly/patternfly/master-dist/dist/tests/filter.html#

Copy link
Member

Choose a reason for hiding this comment

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

that page shows the filter categories with selected items under the Category Filters

Copy link
Member Author

Choose a reason for hiding this comment

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

Going to look at implementing these updates in PF core. Leaving this PR [WIP] until PF core changes are merged.

@dtaylor113 dtaylor113 changed the title feat(filter): update to latest patternfly designs [WIP] feat(filter): update to latest patternfly designs Aug 28, 2018
@dtaylor113
Copy link
Member Author

dtaylor113 commented Aug 29, 2018

@dtaylor113 dtaylor113 force-pushed the filter-updates branch 4 times, most recently from 4faa081 to 9429701 Compare August 29, 2018 20:44
@dtaylor113 dtaylor113 changed the title [WIP] feat(filter): update to latest patternfly designs feat(filter): update to latest patternfly designs Aug 29, 2018
@dtaylor113 dtaylor113 force-pushed the filter-updates branch 2 times, most recently from d6b4823 to bdabe3c Compare August 29, 2018 20:48
display: inline-block;

&>li {
& > li {
Copy link
Member

Choose a reason for hiding this comment

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

this file shouldn't need to be included, rebasing should resolve the issues

Copy link
Member Author

Choose a reason for hiding this comment

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

? I changed this file to get rid of eslint warnings during builds.

Copy link
Member

Choose a reason for hiding this comment

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

it has been fixed already

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, rebased, it did go away -thanks

@dtaylor113
Copy link
Member Author

@priley86 do you have some time to review today? -thanks

@jeff-phillips-18 jeff-phillips-18 requested review from jschuler and removed request for priley86 August 30, 2018 15:24
@jeff-phillips-18 jeff-phillips-18 merged commit 917808a into patternfly:master Aug 30, 2018
@dtaylor113 dtaylor113 deleted the filter-updates branch January 16, 2019 14:39
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

Successfully merging this pull request may close these issues.

5 participants