feat(pfFilter): Add filter options - inline, results count, items label #601
Conversation
jeff-phillips-18
requested review from
cshinn,
cdcabrera,
dtaylor113 and
beanh66
styles/angular-patternfly.less
Outdated
| @@ -1,3 +1,4 @@ | |||
| @import "../node_modules/patternfly/node_modules/bootstrap/less/variables.less"; | |||
There was a problem hiding this comment.
What?! Didn't you stop me from adding bootstrap variables? #543
There was a problem hiding this comment.
LOL, right... I will remove.
test/filters/filter.spec.js
Outdated
| @@ -207,4 +207,51 @@ describe('Directive: pfFilter', function () { | |||
|
|
|||
| expect(element.find('.pf-table-view-selected-label').text()).toContain('0 of 10 selected'); | |||
| }); | |||
|
|
|||
| it('should show results inline when specified', function() { | |||
There was a problem hiding this comment.
To be truly well rounded, each of these three tests need a negative test. Ex: "should not show results inline when not specified" expect(element.find('.filter-pf.inline-filter-pf').length).toBe(0);
There was a problem hiding this comment.
Added negative tests
src/filters/filters.less
Outdated
|
|
||
| .form-group { | ||
| margin-bottom: 0; | ||
| @media (min-width: 345px) { |
There was a problem hiding this comment.
Is there a variable for this? Thought this would be why you @imported bootstrap variables.less?
There was a problem hiding this comment.
Are there no patternfly variables which denote the min-widths at diff screen sizes? If not, we should file an issue and add our own, then we don't have to worry about bootstrap variables being overwritten.
There was a problem hiding this comment.
Removing as this is unnecessary. Thanks.
|
Other than the minor issues I raised, LGTM |
Provides options to: display filter and result on a single line show the total count in the results label (i.e. 5 of 10 Results) change the label from Results to a passed in string
jeff-phillips-18
force-pushed
the
filter
branch
from
4bd1cbe to
a1c44d9
Compare
|
@cdcabrera, ok if I merge this or do you want a chance to review? -thanks |
|
@dtaylor113 taking a look |
There was a problem hiding this comment.
Technical wise lgtm
Behavior feels a little funky around having the field populated then switching to a dropdown filtering method, though that may be just me.
Edit: And so I don't sound like a crazy person... clarification:
I had entered text in the input field then flipped to the month selector not realizing it was a drop down with the associated icon and kept trying to type
Description
Provides options to:
PR Checklist
Rawgit Link (in lieu of screen shots)
https://rawgit.com/jeff-phillips-18/angular-patternfly/filter-docs/dist/docs/index.html#/api/patternfly.filters.component:pfFilter