feat(ExpandCollapse): introduce ExpandCollapse component #613

mareklibra · 2018-09-12T08:08:52Z

What: introduce ExpandCollapse component

Link to Storybook: https://rawgit.com/mareklibra/patternfly-react/ExpandCollapse-sb4/index.html

Additional issues:

Additional info:
Implementation for the Expand Collapse Section is provided.
Patternfly.org Design

The component can be farther used for the "More Information" use-case,
where additional text/components are hidden unless the user clicks
on a link.

Such a per-request rendered content is used for texts exceeding scope of a tooltip or
when the user is expected to copy and paste part of the information.

jeff-phillips-18 · 2018-09-12T14:15:11Z

packages/patternfly-3/patternfly-react/src/components/ExpandCollapse/ExpandCollapse.stories.js

+import { ExpandCollapse } from './index';
+import { ALIGN_TYPES } from './constants';
+
+const stories = storiesOf(`${storybookPackageName(name)}/${STORYBOOK_CATEGORY.WIDGETS}/Expand Collapse`, module);


This should be under STORYBOOK_CATEGORY.FORMS_AND_CONTROLS

jeff-phillips-18 · 2018-09-12T14:18:11Z

packages/patternfly-3/patternfly-react/src/components/ExpandCollapse/ExpandCollapse.stories.js

+);
+
+stories.add(
+  'ExpandCollapse',


Would show the left/center better if we added a div around this with a set width and a border:

<div style={{ width: '400px', border: '1px solid lightgray' }} <ExpandCollapse ...

serenamarie125

@mareklibra the pattern doc shows that the label changes from "Show Information" to "Hide Information". Is that possible with the component?

Also, is it possible to only show the underline on the text (not the spaces)? Here's a screenshot from the storybook which shows the underline below blank spaces

mareklibra · 2018-09-13T10:39:35Z

@serenamarie125 , sure, I will change the underline and add property for text of the expanded state.

Regarding default text values and moving under Forms and Controls:
The component is expected to have multiple use-cases, so far identified are

More Information
or Expand Collapse Section for forms.

I can imagine more coming in the future.

We can either keep the component under the WIDGETS and let the user just provide proper properties or we can implement specialized components for each of these use cases by providing expected texts or alignment. What do you think? @jeff-phillips-18 , @serenamarie125

coveralls · 2018-09-13T11:09:06Z

Pull Request Test Coverage Report for Build 2592

13 of 13 (100.0%) changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.08%) to 80.948%

Totals
Change from base Build 2584:	0.08%
Covered Lines:	3386
Relevant Lines:	3937

💛 - Coveralls

mareklibra · 2018-09-13T11:14:56Z

Changes: the link consists of text only (not the space or icon). Consuming code can provide props for both expanded and collapsed state.

Storybook is updated (but requires cleaning of browser cache).

jeff-phillips-18 · 2018-09-13T12:44:27Z

I think that since the text can be set by the application there is no need for multiple components. I still feel this belongs under Forms and Controls to be consistent with ExpandCollapse pattern found on Patternfly.org. More Information is really just expand/collapse isn't it?

Also, can we set the button's focus state to use outline: 0. The focus treatment is really over done. This should be changed generically but in this case it seems really bad to me. The underline is enough to indicate focus in my opinion.

serenamarie125 · 2018-09-13T18:49:08Z

@mareklibra the storybook link doesn't work for me now. Can you update that so I can approve? As long as we can modify the text & there is the ability to have 2 different labels based on if it's expanded or collapsed we should be fine.

Agree that it likely needs to be in Forms & Controls

mareklibra · 2018-09-14T07:14:33Z

The storybook worked for me once browser's cache is cleared. Anyway, redeployed to a new branch, can you please have a look?

Changes:

moved under Forms & Controls
so default texts changed to Hide/Show Advanced Options
the button has outline:none on focus

Regarding More Information: the difference is just in text and alignment of the link.

serenamarie125 · 2018-09-14T14:52:54Z

@mareklibra I have a couple of more asks ...
1 - the caret should be "selectable" as well as the text
2 - right now the text jumps when the component is selected, I think @jeff-phillips-18 may have some thoughts on why
3 - we should have a line to the right of the link, IMO this should be optional, but shown by default

mareklibra · 2018-09-17T08:29:51Z

@jeff-phillips-18 , thanks, good idea with the use of flex.

jeff-phillips-18

Could you update the storybook please?

packages/patternfly-3/patternfly-react/less/expand-collapse.less

mareklibra · 2018-09-20T07:35:47Z

Storybook updated

jeff-phillips-18 · 2018-09-21T13:36:50Z

@mareklibra I'm still not seeing the updates in the storybook

mareklibra · 2018-10-05T08:12:50Z

@jeff-phillips-18 , please excuse late answer.
The link in the PR description has been updated, storybook should work now.
The PR is rebased.

patternfly-build · 2018-10-05T08:23:50Z

PatternFly-React preview: https://613-pr-patternfly-react-patternfly.surge.sh

serenamarie125

Looks great @mareklibra thanks for the contribution!

priley86 · 2018-10-08T18:30:51Z

hey @mareklibra - this looks excellent to me... I have one suggestion for allowing the consumer to explicitly override the expand/collapse if desired. I think the "controlled" pattern for simple UI state is a nice addition to a component like this. You can see the behavior in my storybook w/ new knob here

Do you mind cherry-picking?

priley86@5bab582

Other than this...PR looks good to go to me!

affects: patternfly-react Implementation for the Expand Collapse Section is provided. The component can be farther used for the "More Information" use-case, where additional text/components are hidden unless the user clicks on a link. Such a per-request rendered content is used for texts exceeding scope of a tooltip or when the user is expected to copy and paste part of the information.

affects: patternfly-react

mareklibra · 2018-10-09T13:51:32Z

@priley86 , thanks, cherry-picked.

Last changes:

rebased
cherry-picked priley86@5bab582
added expanded to propTypes and defaultProps
storybook updated

priley86 · 2018-10-09T14:14:32Z

great! looks good @mareklibra - one last favor... can you update the snapshots? just run yarn test -u and check them in to pass the build...

lizsurette · 2018-10-09T20:54:30Z

@mareklibra I know I'm late to the party but just wanted to say this looks great! Thanks so much for the contribution and looking forward to making use of it :)

The propType and defaultProps added for the "expanded" property.

mareklibra · 2018-10-15T09:30:35Z

Thanks for the nice feedback :-) I have updated the test snapshot for the expanded prop.

janwright73 added the review label Sep 12, 2018

mareklibra mentioned this pull request Sep 12, 2018

feat(MoreInformation): introduce the MoreInformation compoment #602

Closed

jeff-phillips-18 added ux review labels Sep 12, 2018

jeff-phillips-18 reviewed Sep 12, 2018

View reviewed changes

serenamarie125 suggested changes Sep 13, 2018

View reviewed changes

mareklibra force-pushed the ExpandCollapse branch 2 times, most recently from 0415ecc to 092e5c9 Compare September 13, 2018 11:06

mareklibra force-pushed the ExpandCollapse branch 2 times, most recently from 08112c3 to b8cbf55 Compare September 14, 2018 07:08

mareklibra force-pushed the ExpandCollapse branch 2 times, most recently from 898fa57 to 019bb66 Compare September 14, 2018 07:23

jeff-phillips-18 reviewed Sep 18, 2018

View reviewed changes

packages/patternfly-3/patternfly-react/less/expand-collapse.less Show resolved Hide resolved

mareklibra force-pushed the ExpandCollapse branch from e61e689 to ec6c55b Compare September 20, 2018 07:35

priley86 added the enhancement 🚀 label Oct 2, 2018

mareklibra force-pushed the ExpandCollapse branch from ec6c55b to a507208 Compare October 5, 2018 08:12

mareklibra mentioned this pull request Oct 5, 2018

feat(DesktopViewer): introduce DesktopViewer component #607

Merged

jeff-phillips-18 added css approved and removed css review labels Oct 5, 2018

serenamarie125 previously approved these changes Oct 6, 2018

View reviewed changes

serenamarie125 added ux approved and removed ux review labels Oct 7, 2018

jeff-phillips-18 previously approved these changes Oct 8, 2018

View reviewed changes

priley86 mentioned this pull request Oct 8, 2018

ListView: add expanded prop to ListViewItem #691

Closed

mareklibra and others added 3 commits October 9, 2018 15:32

icon alignment and bordered option

e0bfb13

test(ExpandCollapse): add test for bordered ExpandCollapse

52810ab

affects: patternfly-react

mareklibra dismissed stale reviews from jeff-phillips-18 and serenamarie125 via 52810ab October 9, 2018 13:33

mareklibra force-pushed the ExpandCollapse branch from a507208 to 52810ab Compare October 9, 2018 13:33

fix(expand-collapse): controlled expand collapse pattern

d7c5a52

jeff-phillips-18 previously approved these changes Oct 10, 2018

View reviewed changes

fix(ExpandCollapse): add propType for "expanded"

763b622

The propType and defaultProps added for the "expanded" property.

mareklibra dismissed jeff-phillips-18’s stale review via 763b622 October 15, 2018 09:29

mareklibra force-pushed the ExpandCollapse branch from edcb4b8 to 763b622 Compare October 15, 2018 09:29

priley86 approved these changes Oct 15, 2018

View reviewed changes

jeff-phillips-18 approved these changes Oct 15, 2018

View reviewed changes

jeff-phillips-18 merged commit 31eac90 into patternfly:master Oct 15, 2018

feat(ExpandCollapse): introduce ExpandCollapse component #613

feat(ExpandCollapse): introduce ExpandCollapse component #613

Uh oh!

Conversation

mareklibra commented Sep 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jeff-phillips-18 Sep 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeff-phillips-18 Sep 12, 2018

Choose a reason for hiding this comment

Uh oh!

mareklibra Sep 13, 2018

Choose a reason for hiding this comment

Uh oh!

serenamarie125 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mareklibra commented Sep 13, 2018

Uh oh!

coveralls commented Sep 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 2592

💛 - Coveralls

Uh oh!

mareklibra commented Sep 13, 2018

Uh oh!

jeff-phillips-18 commented Sep 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serenamarie125 commented Sep 13, 2018

Uh oh!

mareklibra commented Sep 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serenamarie125 commented Sep 14, 2018

Uh oh!

mareklibra commented Sep 17, 2018

Uh oh!

jeff-phillips-18 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mareklibra commented Sep 20, 2018

Uh oh!

jeff-phillips-18 commented Sep 21, 2018

Uh oh!

mareklibra commented Oct 5, 2018

Uh oh!

patternfly-build commented Oct 5, 2018

Uh oh!

serenamarie125 left a comment

Choose a reason for hiding this comment

Uh oh!

priley86 commented Oct 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mareklibra commented Oct 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

priley86 commented Oct 9, 2018

Uh oh!

lizsurette commented Oct 9, 2018

Uh oh!

mareklibra commented Oct 15, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

mareklibra commented Sep 12, 2018 •

edited

Loading

jeff-phillips-18 Sep 12, 2018 •

edited

Loading

serenamarie125 left a comment •

edited

Loading

coveralls commented Sep 13, 2018 •

edited

Loading

jeff-phillips-18 commented Sep 13, 2018 •

edited

Loading

mareklibra commented Sep 14, 2018 •

edited

Loading

priley86 commented Oct 8, 2018 •

edited

Loading

mareklibra commented Oct 9, 2018 •

edited

Loading