Fixes #881 - Add LoadingState component #886

xprazak2 · 2018-11-06T06:53:09Z

What: Adds new LoadingState component

Additional issues: none

coveralls · 2018-11-06T07:27:02Z

Pull Request Test Coverage Report for Build 3318

11 of 12 (91.67%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.03%) to 81.35%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
packages/patternfly-3/patternfly-react/src/components/LoadingState/LoadingState.js	11	12	91.67%

Totals
Change from base Build 3317:	0.03%
Covered Lines:	3887
Relevant Lines:	4503

💛 - Coveralls

patternfly-build · 2018-11-06T08:38:14Z

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

jeff-phillips-18 · 2018-11-07T13:25:38Z

Thanks for the PR @xprazak2. I'd like to see this in patternfly first then added here to avoid the extra sass in this repo. Would you mind creating a PR there first? Or I can do that relatively quickly and I have some cycles. Just trying to keep the css in this repo to a minimum for patternfly patterns.

jeff-phillips-18 · 2018-11-07T15:17:31Z

Took the liberty of putting up the PR in patternfly: patternfly/patternfly#1152

xprazak2 · 2018-11-08T10:07:27Z

I removed the stylesheets and opened PR for patternfly repo.

And then closed it again when I noticed @jeff-phillips-18 beat me to it 😄

jeff-phillips-18

Need to bump the version of patternfly to 3.56

jeff-phillips-18 · 2018-11-08T13:28:13Z

packages/patternfly-3/patternfly-react/src/components/LoadingState/LoadingState.js

+    const spinner = (
+      <div className="pf-loading-state">
+        <Spinner loading={loading} size="lg" />
+        <p>{loadingText}</p>


the <p> can be removed

jeff-phillips-18 · 2018-11-08T13:31:25Z

packages/patternfly-3/patternfly-react/src/components/LoadingState/LoadingState.js

+  loadingText: PropTypes.string,
+  children: PropTypes.node,
+  timeout: PropTypes.number
+};


the propTypes need comments to add descriptions in storybook.

This should take a size for the entire loading state (rather than spinnerSize) and add loading-state-pf-[lg, sm, xs] to the outer div and set the spinner size appropriately

jeff-phillips-18 · 2018-11-08T13:32:45Z

packages/patternfly-3/patternfly-react/src/components/LoadingState/LoadingState.js

+    const { loading, loadingText, children } = this.props;
+    const spinner = (
+      <div className="pf-loading-state">
+        <Spinner loading={loading} size="lg" />


we should allow a prop to set the size and add the appropriate class to the pf-loading-state as well as set the size on the Spinner.

dmiller9911 · 2018-11-08T13:57:55Z

packages/patternfly-3/patternfly-react/src/components/LoadingState/LoadingState.js

+  }
+
+  componentDidMount() {
+    setTimeout(() => {


Need to set this into a property and clear it if the component unmounts before the function is fired.

xprazak2 · 2018-11-12T13:12:51Z

I bumped the patternfly version, updated the props and do a cleanup in componentWillUnmount.

When running storybook locally, the new styles were not applied to the component. patternfly/patternfly#1154 should take care of that.

jeff-phillips-18 · 2018-11-14T16:22:32Z

packages/patternfly-3/patternfly-react/package.json

    "css-element-queries": "^1.0.1",
-    "patternfly": "^3.52.4",
+    "patternfly": "^3.57.1",
    "react-bootstrap": "^0.32.1",


You also need to update packages/patternfly-3/patternfly-react-extensions/package.json to use the same version

jeff-phillips-18 · 2018-11-14T16:25:45Z

packages/patternfly-3/patternfly-react/src/components/LoadingState/LoadingState.js

+  loadingText: PropTypes.string,
+  children: PropTypes.node,
+  timeout: PropTypes.number
+};


This should take a size for the entire loading state (rather than spinnerSize) and add loading-state-pf-[lg, sm, xs] to the outer div and set the spinner size appropriately

jeff-phillips-18 · 2018-11-14T16:26:25Z

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

+      story
+    });
+  })
+);


Would be nice to add a knob to set the size

jeff-phillips-18 · 2018-11-14T16:26:43Z

packages/patternfly-3/patternfly-react/src/components/LoadingState/LoadingState.test.js

+  jest.runAllTimers();
+  component.update();
+  expect(toJson(component.render())).toMatchSnapshot();
+});


Add tests for sizes.

xprazak2 · 2018-11-15T08:24:10Z

The size prop is now applied to the whole component, not just spinner and there is a test for that. I added knob for sizes and bumped the patternfly version for both patternfly-react-extensions and react-console so that 3.52.z version is purged from yarn.lock on yarn upgrade and the styles from 3.57.1 are applied in storybook.

jeff-phillips-18 · 2018-11-15T11:44:07Z

packages/patternfly-3/patternfly-react/src/components/LoadingState/LoadingState.js

+  /** delay in showing the children */
+  timeout: PropTypes.number,
+  /** size of the spinner */
+  size: PropTypes.oneOf(['lg', 'sm', 'xs']),


Need to add 'md' here as well (though it doesn't really add styling except to give the default).

jeff-phillips-18 · 2018-11-15T11:44:54Z

packages/patternfly-3/patternfly-react/src/components/LoadingState/LoadingState.js

+    const { loading, loadingText, children, size, additionalClasses } = this.props;
+
+    const spinner = (
+      <div className={`loading-state-pf loading-state-pf-${size} ${additionalClasses}`}>


Better to use classnames

xprazak2 · 2018-11-16T10:10:01Z

I added md and use classnames.

xprazak2 · 2018-11-19T09:37:08Z

Fixing the linter offence.

jeff-phillips-18 · 2018-11-19T11:26:10Z

@xprazak2 Looks like you need to do a snapshot update.

xprazak2 · 2018-11-20T13:26:28Z

I rebased and updated the snapshots.

mfrances17

LGTM

xprazak2 force-pushed the loading-state branch from 69338d6 to 08cd1a3 Compare November 6, 2018 07:15

ohadlevy mentioned this pull request Nov 6, 2018

Fixes #24041 - Move HW Models Table to pf-react theforeman/foreman#5755

Merged

xprazak2 force-pushed the loading-state branch from 08cd1a3 to 7fbfe12 Compare November 6, 2018 08:24

xprazak2 force-pushed the loading-state branch from 7fbfe12 to 50bd5d2 Compare November 8, 2018 10:06

xprazak2 force-pushed the loading-state branch from 50bd5d2 to fc65848 Compare November 8, 2018 10:08

jeff-phillips-18 requested changes Nov 8, 2018

View reviewed changes

dmiller9911 reviewed Nov 8, 2018

View reviewed changes

xprazak2 force-pushed the loading-state branch 3 times, most recently from 4c2cc56 to 8caffcd Compare November 12, 2018 13:10

xprazak2 force-pushed the loading-state branch 2 times, most recently from f7190dc to 3526a8c Compare November 13, 2018 10:09

jeff-phillips-18 requested changes Nov 14, 2018

View reviewed changes

xprazak2 force-pushed the loading-state branch 2 times, most recently from 0d194d7 to d8599df Compare November 15, 2018 08:17

jeff-phillips-18 reviewed Nov 15, 2018

View reviewed changes

xprazak2 force-pushed the loading-state branch 2 times, most recently from 30c1170 to 7e5c0bb Compare November 16, 2018 10:07

jeff-phillips-18 previously approved these changes Nov 16, 2018

View reviewed changes

xprazak2 dismissed jeff-phillips-18’s stale review via c85cf07 November 19, 2018 09:35

xprazak2 force-pushed the loading-state branch from 7e5c0bb to c85cf07 Compare November 19, 2018 09:35

xprazak2 force-pushed the loading-state branch from c85cf07 to 7ff63b5 Compare November 20, 2018 13:25

feat(LoadingState): Add LoadingState component (patternfly#881)

a498176

xprazak2 force-pushed the loading-state branch from 7ff63b5 to a498176 Compare November 21, 2018 08:51

jeff-phillips-18 approved these changes Nov 27, 2018

View reviewed changes

jeff-phillips-18 requested review from dlabaj and mfrances17 November 27, 2018 16:37

dlabaj approved these changes Nov 27, 2018

View reviewed changes

dlabaj merged commit d48d45f into patternfly:master Nov 27, 2018

mfrances17 reviewed Nov 27, 2018

View reviewed changes

Fixes #881 - Add LoadingState component #886

Fixes #881 - Add LoadingState component #886

Uh oh!

Conversation

xprazak2 commented Nov 6, 2018

Uh oh!

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

Uh oh!

Pull Request Test Coverage Report for Build 3318

💛 - Coveralls

Uh oh!

patternfly-build commented Nov 6, 2018

Uh oh!

jeff-phillips-18 commented Nov 7, 2018

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

jeff-phillips-18 left a comment

Choose a reason for hiding this comment

Uh oh!

jeff-phillips-18 Nov 8, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xprazak2 commented Nov 12, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xprazak2 commented Nov 15, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xprazak2 commented Nov 16, 2018

Uh oh!

xprazak2 commented Nov 19, 2018

Uh oh!

jeff-phillips-18 commented Nov 19, 2018

Uh oh!

xprazak2 commented Nov 20, 2018

Uh oh!

mfrances17 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

coveralls commented Nov 6, 2018 •

edited

Loading

jeff-phillips-18 commented Nov 7, 2018 •

edited

Loading

xprazak2 commented Nov 8, 2018 •

edited

Loading

jeff-phillips-18 Nov 8, 2018 •

edited

Loading