fix(Progress): do not show progress without progress or label props (S…

…emantic-Org#1355) * fix(Progress): do not show progress without progress or label props * style(Progress): some style fixes * style(Progress): update typings * Update Progress.js
harel · Feb 25, 2017 · da163cd · da163cd
1 parent 10f5547
commit da163cd
Show file tree

Hide file tree

Showing 5 changed files with 69 additions and 61 deletions.
diff --git a/docs/app/Examples/modules/Progress/Variations/ProgressExampleInverted.js b/docs/app/Examples/modules/Progress/Variations/ProgressExampleInverted.js
@@ -3,16 +3,16 @@ import { Progress, Segment } from 'semantic-ui-react'
 
 const ProgressExampleInverted = () => (
   <Segment inverted>
-    <Progress percent={21} inverted label>
+    <Progress percent={21} inverted progress>
       Uploading Files
     </Progress>
-    <Progress percent={100} inverted label success>
+    <Progress percent={100} inverted progress success>
       success
     </Progress>
-    <Progress percent={100} inverted label warning>
+    <Progress percent={100} inverted progress warning>
       warning
     </Progress>
-    <Progress percent={100} inverted label error>
+    <Progress percent={100} inverted progress error>
       error
     </Progress>
   </Segment>

diff --git a/docs/app/Examples/modules/Progress/Variations/ProgressExampleInvertedColor.js b/docs/app/Examples/modules/Progress/Variations/ProgressExampleInvertedColor.js
@@ -3,19 +3,19 @@ import { Progress, Segment } from 'semantic-ui-react'
 
 const ProgressExampleInvertedColor = () => (
   <Segment inverted>
-    <Progress percent={32} inverted color='red' label />
-    <Progress percent={59} inverted color='orange' label />
-    <Progress percent={13} inverted color='yellow' label />
-    <Progress percent={37} inverted color='olive' label />
-    <Progress percent={83} inverted color='green' label />
-    <Progress percent={23} inverted color='teal' label />
-    <Progress percent={85} inverted color='blue' label />
-    <Progress percent={38} inverted color='violet' label />
-    <Progress percent={47} inverted color='purple' label />
-    <Progress percent={29} inverted color='pink' label />
-    <Progress percent={68} inverted color='brown' label />
-    <Progress percent={36} inverted color='grey' label />
-    <Progress percent={72} inverted color='black' label />
+    <Progress percent={32} inverted color='red' progress />
+    <Progress percent={59} inverted color='orange' progress />
+    <Progress percent={13} inverted color='yellow' progress />
+    <Progress percent={37} inverted color='olive' progress />
+    <Progress percent={83} inverted color='green' progress />
+    <Progress percent={23} inverted color='teal' progress />
+    <Progress percent={85} inverted color='blue' progress />
+    <Progress percent={38} inverted color='violet' progress />
+    <Progress percent={47} inverted color='purple' progress />
+    <Progress percent={29} inverted color='pink' progress />
+    <Progress percent={68} inverted color='brown' progress />
+    <Progress percent={36} inverted color='grey' progress />
+    <Progress percent={72} inverted color='black' progress />
   </Segment>
 )
 

diff --git a/src/modules/Progress/Progress.js b/src/modules/Progress/Progress.js
@@ -3,6 +3,7 @@ import _ from 'lodash'
 import React, { Component, PropTypes } from 'react'
 
 import {
+  createShorthand,
   customPropTypes,
   getElementType,
   getUnhandledProps,
@@ -51,16 +52,7 @@ class Progress extends Component {
     inverted: PropTypes.bool,
 
     /** Can be set to either to display progress as percent or ratio. */
-    label: customPropTypes.every([
-      customPropTypes.some([
-        customPropTypes.demand(['percent']),
-        customPropTypes.demand(['total', 'value']),
-      ]),
-      PropTypes.oneOfType([
-        PropTypes.bool,
-        PropTypes.oneOf(['ratio', 'percent']),
-      ]),
-    ]),
+    label: customPropTypes.itemShorthand,
 
     /** Current percent complete. */
     percent: customPropTypes.every([
@@ -75,7 +67,10 @@ class Progress extends Component {
     precision: PropTypes.number,
 
     /** A progress bar can contain a text value indicating current progress. */
-    progress: PropTypes.bool,
+    progress: PropTypes.oneOfType([
+      PropTypes.bool,
+      PropTypes.oneOf(['percent', 'ratio']),
+    ]),
 
     /** A progress bar can vary in size. */
     size: PropTypes.oneOf(_.without(SUI.SIZES, 'mini', 'huge', 'massive')),
@@ -139,29 +134,41 @@ class Progress extends Component {
     return autoSuccess && (percent >= 100 || value >= total)
   }
 
-  showProgress = () => {
-    const { label, precision, progress, total, value } = this.props
+  renderLabel = () => {
+    const { children, label } = this.props
 
-    if (label || progress || !_.isUndefined(precision)) return true
-    return !(_.every([total, value], _.isUndefined))
+    if (!_.isNil(children)) return <div className='label'>{children}</div>
+    return createShorthand('div', val => ({ children: val }), label, { className: 'label' })
+  }
+
+  renderProgress = percent => {
+    const {
+      precision,
+      progress,
+      total,
+      value,
+    } = this.props
+
+    if (!progress && _.isUndefined(precision)) return
+    return (
+      <div className='progress'>
+        { progress !== 'ratio' ? `${percent}%` : `${value}/${total}` }
+      </div>
+    )
   }
 
   render() {
     const {
       active,
       attached,
-      children,
       className,
       color,
       disabled,
       error,
       indicating,
       inverted,
-      label,
       size,
       success,
-      total,
-      value,
       warning,
     } = this.props
 
@@ -182,19 +189,14 @@ class Progress extends Component {
     )
     const rest = getUnhandledProps(Progress, this.props)
     const ElementType = getElementType(Progress, this.props)
-
     const percent = this.getPercent()
 
     return (
       <ElementType {...rest} className={classes}>
         <div className='bar' style={{ width: `${percent}%` }}>
-          {this.showProgress() && (
-            <div className='progress'>
-              { label !== 'ratio' ? `${percent}%` : `${value}/${total}` }
-            </div>
-          )}
+          {this.renderProgress(percent)}
         </div>
-        {children && <div className='label'>{children}</div>}
+        {this.renderLabel()}
       </ElementType>
     )
   }

diff --git a/src/modules/Progress/index.d.ts b/src/modules/Progress/index.d.ts
@@ -38,7 +38,7 @@ interface ProgressProps {
   inverted?: string;
 
   /** Can be set to either to display progress as percent or ratio. */
-  label?: boolean | 'ratio' | 'percent';
+  label?: any;
 
   /** Current percent complete. */
   percent?: number | string;
@@ -47,7 +47,7 @@ interface ProgressProps {
   precision?: number;
 
   /** A progress bar can contain a text value indicating current progress. */
-  progress?: boolean;
+  progress?: boolean | 'percent' | 'ratio';
 
   /** A progress bar can vary in size. */
   size?: 'tiny' | 'small' | 'medium' | 'large' | 'big';

diff --git a/test/specs/modules/Progress/Progress-test.js b/test/specs/modules/Progress/Progress-test.js
@@ -96,22 +96,11 @@ describe('Progress', () => {
   })
 
   describe('label', () => {
-    it('displays the progress as a percentage by default', () => {
-      shallow(<Progress percent={20} label />)
-        .should.have.descendants('.progress')
-        .and.contain.text('20%')
-    })
-    it('displays the progress as a ratio when set to "ratio"', () => {
-      shallow(<Progress label='ratio' value={1} total={2} />)
+    it('shows the label text when provided', () => {
+      shallow(<Progress label='some-label' />)
         .children()
-        .find('.progress')
-        .should.contain.text('1/2')
-    })
-    it('displays the progress as a percentage when set to "percent"', () => {
-      shallow(<Progress label='percent' value={1} total={2} />)
-        .children()
-        .find('.progress')
-        .should.contain.text('50%')
+        .find('.label')
+        .should.contain.text('some-label')
     })
   })
 
@@ -131,6 +120,23 @@ describe('Progress', () => {
         .find('.bar')
         .should.not.have.descendants('.progress')
     })
+    it('displays the progress as a percentage by default', () => {
+      shallow(<Progress percent={20} progress />)
+        .should.have.descendants('.progress')
+        .and.contain.text('20%')
+    })
+    it('displays the progress as a ratio when set to "ratio"', () => {
+      shallow(<Progress progress='ratio' value={1} total={2} />)
+        .children()
+        .find('.progress')
+        .should.contain.text('1/2')
+    })
+    it('displays the progress as a percentage when set to "percent"', () => {
+      shallow(<Progress progress='percent' value={1} total={2} />)
+        .children()
+        .find('.progress')
+        .should.contain.text('50%')
+    })
     it('shows the percent complete', () => {
       shallow(<Progress percent={72} progress />)
         .children()
@@ -191,7 +197,7 @@ describe('Progress', () => {
 
   describe('total/value', () => {
     it('calculates the percent complete', () => {
-      shallow(<Progress value={1} total={2} />)
+      shallow(<Progress value={1} total={2} progress />)
         .children()
         .find('.progress')
         .should.contain.text('50%')