Update Statistic to v1 API #334

layershifter · 2016-07-15T08:33:41Z

API

Types

Statistic

A statistic can display a value with a label above or below it.

<div class="ui statistic">
  <div class="value">5,550</div>
  <div class="label">Downloads</div>
</div>
<div class="ui statistic">
  <div class="label">Views</div>
  <div class="value">40,509</div>
</div>

<Statistic>
  <Statistic.Value>5,550</Statistic.Value>
  <Statistic.Label>Downloads</Statistic.Label>
</Statistic>
<Statistic>
  <Statistic.Label>Views</Statistic.Label>
  <Statistic.Value>40,509</Statistic.Value>
</Statistic>

Statistic Group

A group of statistics

<div class="ui statistics">
  <div class="statistic">
    <div class="value">22</div>
    <div class="label">Faves</div>
  </div>
  ...
</div>

<Statistic.Group>
  <Statistic>
    <Statistic.Value>22</Statistic.Value>
    <Statistic.Label>Faves</Statistic.Label>
  </Statistic>
  ...
</Statistic.Group>

Content

Value

A statistic can contain a numeric, icon, image, or text value

<div class="statistic">
    <div class="value">22</div>
   <div class="label">Saves</div>
</div>
<div class="statistic">
  <div class="text value">Three<br>Thousand</div>
  <div class="label">Signups</div>
</div>
<div class="statistic">
  <div class="value"><i class="plane icon"></i> 5</div>
  <div class="label">Flights</div>
</div>
<div class="statistic">
    <div class="value">
      <img src="/images/avatar/small/joe.jpg" class="ui circular inline image"> 42
    </div>
    <div class="label">Team Members</div>
</div>

<Statistic>
  <Statistic.Value>22</Statistic.Value>
  <Statistic.Label>Saves</Statistic.Label>
</Statistic>
<Statistic>
  <Statistic.Value text>Three<br>Thousand</Statistic.Value>
  <Statistic.Label>Signups</Statistic.Label>
</Statistic>
<Statistic>
  <Statistic.Value><Icon name='plane' /> 5</Statistic.Value>
  <Statistic.Label>Flights</Statistic.Label>
</Statistic>
<Statistic>
  <Statistic.Value><Image src='/images/avatar/small/joe.jpg' ... /></Statistic.Value>
  <Statistic.Label>Team Members</Statistic.Label>
</Statistic>

Label

A statistic can contain a label to help provide context for the presented value

Already showed

Variations

Horizontal Statistic

A statistic can present its measurement horizontally

<div class="ui horizontal statistic">...</div>
<div class="ui horizontal statistics">...</div>

<Statistic horizontal>...</Statistic>
<Statistic.Group horizontal>...</Statistic.Group>

Colored

A statistic can be formatted to be different colors

<div class="red statistic"></div>

<Statistic color='red'>...</Statistic>

Colors will be used from semanticUtils

Inverted

A statistic can be formatted to fit on a dark background

<div class="ui inverted statistic">...</div>

<Statistic inverted>...</Statistic>

Evenly Divided

A statistic group can have its items divided evenly

Will be taken from #183

Floated

An statistic can sit to the left or right of other content

<div class="ui right floated statistic">...</div>

<Statistic floated='left'>...</Statistic>
<Statistic floated='right'>...</Statistic>

Size

A statistic can vary in size

<Statistic size='mini'>...</Statistic>

There is no medium, big and massive sizes.

…into feature/rail

layershifter · 2016-07-15T08:34:25Z

Refers #269

layershifter · 2016-07-15T14:54:24Z

What about this?

<Statistic label='Faves' value='22' />

codecov-io · 2016-07-15T14:55:15Z

Current coverage is 92.47%

Merging #334 into master will increase coverage by 0.96%

@@             master       #334   diff @@
==========================================
  Files            62         62          
  Lines           777        797    +20   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits            711        737    +26   
+ Misses           66         60     -6   
  Partials          0          0

Powered by Codecov. Last updated by c5b0425...c9cc2f2

levithomason · 2016-07-15T15:31:30Z

API looks great. The simplified props version look right as well:

<Statistic label='Faves' value='22' />

We probably just accept the limitation that using props only like this will generate markup with the value on top and the label on bottom. I assume this is the most common configuration.

Any intuitive ideas on how to allow reordering label/value with the props API?

layershifter · 2016-07-17T08:56:36Z

Any intuitive ideas on how to allow reordering label/value with the props API?

What about boolean reverse/labeled/valued prop?

into fix/statistic

layershifter · 2016-07-17T09:12:07Z

What about short syntax for label and value?

<Statistic.Label text='Faves' />
<Statistic.Value text='22' />

UPD:
Oops, text prop is already taken in Statistic.Value, what about content?

…into fix/statistic

layershifter · 2016-07-17T12:22:49Z

I've implemented all features, added tests and docs. @levithomason wait for feedback 😄

levithomason · 2016-07-17T15:13:03Z

What about boolean reverse/labeled/valued prop?

After thinking it through, I think we should just provide a single layout, value on top, label on bottom. If a user wants more control, they can use the sub components. This seems to be the cleanest solution, for now.

Oops, text prop is already taken in Statistic.Value, what about content?

Nice catch, I never saw the text option buried in the docs. We've been using text in many places for shorthand configuration, which as you've noted will not work here. I'd like for shorthand props to be as consistent as possible between all components. My worry is that since content is also a common SUI term, this will have conflicts elsewhere. This makes me think we should not provide shorthand props for label/value sub components here just yet.

Perhaps add a TODO in each component to find a shorthand prop for each that is not text but that is also consistent with shorthand props in other components. Then, once we get through the v1 updates, we can scan through the components and find the most appropriate prop name.

levithomason · 2016-07-17T15:31:04Z

Some more ideas on props for Statistic sub components. The Statistic already accepts label and value shorthand props. We could allow these props on the subcomponents:

<Statistic.Value value={99} />
<Statistic.Label label='Likes' />

Regarding label/value order for Statistic shorthand props, React props are order dependent. As noted in the React docs:

NOTE:
Order matters. By putting the {...other} before your JSX props you ensure that the consumer of your component can't override them. In the example above we have guaranteed that the input will be of type "checkbox".

So we could simply do this:

<Statistic label='Label first' value={3} />
<Statistic value={99} label='Label last' />

See it in action here: http://www.react.run/HJxagQKP/2

levithomason · 2016-07-17T16:09:40Z

src/views/Statistic/Statistic.js

+  return (<div className={classes} {...rest}>
+    <Statistic.Value text={text}>{value}</Statistic.Value>
+    <Statistic.Label>{label}</Statistic.Label>
+  </div>)


Since props are applied in order, we need to spread the rest before applying the className. The classes includes the user's className values. If we apply the rest after the classes, then their classNames will override the classes we've built up.

Also, I think the linter should have caught this but let's return multiline JSX like this:

return ( <div {...rest} className={classes}> <Statistic.Value text={text}>{value}</Statistic.Value> <Statistic.Label>{label}</Statistic.Label> </div> )

https://facebook.github.io/react/docs/transferring-props.html#consuming-and-transferring-the-same-prop

levithomason · 2016-07-17T16:14:48Z

src/views/Statistic/StatisticGroup.js

+    itemsJSX.push(<Statistic key={key} {...item} />)
+  })
+
+  return <div className={classes} {...rest}>{itemsJSX}</div>


As noted in Statistic, spread before applying className to prevent user's className from overriding the className build up.

Conformance tests ensure props are spread, I should add a test to ensure className is not override during spread ;)

Have to correct myself here, while this is true regarding prop order rest actually does not include className in this example. That is because the className prop is being handled in the propTypes, so getUnhandledProps is returning an object of props that does not include className.

Let's still spread them first, and I'm adding the common test now. I just wanted to point out that this would actually work as is and my original comment was incorrect.

layershifter · 2016-07-17T16:34:41Z

Okay, here is some problems:

common.isConformant(StatisticGroup) fails, seems test might be updated:
common.propValueOnlyToClassName(StatisticGroup, 'widths') fails, seems there needed new test in commonTests:

…into fix/statistic

levithomason · 2016-07-17T16:40:08Z

test/specs/views/Stastistic/StatisticGroup-test.js

 import StatisticGroup from 'src/views/Statistic/StatisticGroup'

 describe('StatisticGroup', () => {
  common.isConformant(StatisticGroup)
  common.hasUIClassName(StatisticGroup)
  common.rendersChildren(StatisticGroup)
  common.propKeyOnlyToClassName(StatisticGroup, 'horizontal')
+  common.propValueOnlyToClassName(StatisticGroup, 'widths')


This test is mostly accurate, but not entirely. Consider when a user passes widths='3', the value '3' is not actually added to the className, the word three is.

We should likely add a common test for implementsNumberToWordProp(Statistic, 'widths'). This test would assert that the numberToWord value is added to the className. It would also assert that it works with numbers, strings, and number words (i.e. 'two').

The reason I think we should allow the second param to this common tests is that on other components, like FormFields, the numerToWord prop is width. This way, we can point this test to a specific prop name.

layershifter · 2016-07-17T17:03:30Z

I've made sort, seems it make all ugly 😿

…into fix/statistic

layershifter · 2016-07-17T17:46:38Z

I've created implementsNumberToWordProp, take a look

Need fix for common.isConformant(StatisticGroup) and decision for props order

levithomason · 2016-07-17T18:04:20Z

I've made sort, seems it make all ugly

Oh boy, yep. That isn't good nor intuitive. Let's not sort.

levithomason · 2016-07-17T18:08:27Z

I'll be tied up the next couple hours. Will check on the rest then. Thanks much for all the refactors and rapid responses!

layershifter · 2016-07-18T15:21:09Z

It seems that componentClassName definition in componentInfo needs to be modified for #203

levithomason · 2016-07-18T15:22:12Z

Whoops, missed this one. Checking...

…into fix/statistic

levithomason · 2016-07-18T15:41:05Z

Rebase and give this a shot. TechnologyAdvice/stardust@c5b0425

Tested on this branch and master. It uses the plural name of the parent for componentClassName on sub component groups. So, statistics for StatisticGroup.

…into fix/statistic

layershifter · 2016-07-18T15:51:28Z

Seems it works 👍 Waiting for review

levithomason · 2016-07-18T15:57:27Z

On it

* (feat) Rail Semantic-Org#181 * (feat) Rail Semantic-Org#181 * (feat) Rail docs Semantic-Org#181 * (fix) Sort Rail props Semantic-Org#181 * (fix) Rail review fixes Semantic-Org#181 * (fix) Rail review fixes Semantic-Org#181 * (fix) Rail review fixes Semantic-Org#181 * (fix) Rail sizes Semantic-Org#181 * (fix) Rail sizes in docs Semantic-Org#181 * (fix) Statistic * (feat) Statistic Item * (feat) Statistic Label and Value * (feat) Statistic Group * (fix) Statistic fix sizes * (fix) Statistic functionality * (fix) Statistic docs * (fix) Statistic lint fixes * (fix) Statistic tests * (Statistic) Remove shorthands * (utils) Update numberToWord * (Statistic) Fix returned JSX * (utils) Update numberToWord, fix imports * (Statistic) Update widths usage * (Statistic) Fix JSDoc * (Statistic) Update `rest` prop * (utils) implementsNumberToWordProp test * (Statistic) Test update * fix(Statistic) Fix example

Alexander Fedyashov and others added 11 commits July 12, 2016 18:23

(feat) Rail Semantic-Org#181

95cfe48

(feat) Rail Semantic-Org#181

d2cfef8

(feat) Rail docs Semantic-Org#181

97aac53

(fix) Sort Rail props Semantic-Org#181

d9c324b

Merge branch 'master' of /~https://github.com/TechnologyAdvice/stardust …

8f7c2e5

…into feature/rail

(fix) Rail review fixes Semantic-Org#181

7db8305

(fix) Rail review fixes Semantic-Org#181

bd296b5

(fix) Rail review fixes Semantic-Org#181

c3682a0

(fix) Rail sizes Semantic-Org#181

d74e4f6

(fix) Rail sizes in docs Semantic-Org#181

3127c4a

(fix) Statistic

beb89fa

Alexander Fedyashov added 3 commits July 15, 2016 17:11

(feat) Statistic Item

67505e3

(feat) Statistic Label and Value

ac8560c

(feat) Statistic Group

6d0aeec

(fix) Statistic fix sizes

3ab3808

Merge branch 'fix/statistic' of /~https://github.com/layershifter/stardust

5a66ab2

into fix/statistic

layershifter added 5 commits July 17, 2016 14:12

(fix) Statistic functionality

752fb69

(fix) Statistic docs

31af031

(fix) Statistic lint fixes

a6feb32

(fix) Statistic tests

0bc7db9

Merge branch 'master' of /~https://github.com/TechnologyAdvice/stardust …

0d1da2f

…into fix/statistic

(utils) Update numberToWord

c7fbb20

levithomason reviewed Jul 17, 2016
View reviewed changes

(Statistic) Fix returned JSX

5f6fa79

levithomason reviewed Jul 17, 2016
View reviewed changes

layershifter added 3 commits July 17, 2016 19:20

(utils) Update numberToWord, fix imports

4de5f51

(Statistic) Update widths usage

3c9634b

(Statistic) Fix JSDoc

4d7a47e

layershifter added 2 commits July 17, 2016 19:36

Merge branch 'master' of /~https://github.com/TechnologyAdvice/stardust …

b07c88d

…into fix/statistic

(Statistic) Update rest prop

277c8b0

levithomason reviewed Jul 17, 2016
View reviewed changes

layershifter added 3 commits July 17, 2016 20:42

(utils) implementsNumberToWordProp test

7ff0c77

(Statistic) Test update

574b05c

Merge branch 'master' of /~https://github.com/TechnologyAdvice/stardust …

2bbeb26

…into fix/statistic

Alexander Fedyashov added 2 commits July 18, 2016 18:22

Merge branch 'master' of /~https://github.com/TechnologyAdvice/stardust …

6e851ff

…into fix/statistic

fix(Statistic) Fix example

48ac26c

Merge branch 'master' of /~https://github.com/TechnologyAdvice/stardust …

c9cc2f2

…into fix/statistic

levithomason merged commit 9fcaae2 into Semantic-Org:master Jul 18, 2016

levithomason mentioned this pull request Jul 18, 2016

Update legacy component APIs to the v1 API #269

Closed

14 tasks

dvdzkwsk mentioned this pull request Jul 22, 2016

Rename plural group components (Fields) -> Group #203

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Statistic to v1 API #334

Update Statistic to v1 API #334

layershifter commented Jul 15, 2016

layershifter commented Jul 15, 2016

layershifter commented Jul 15, 2016

codecov-io commented Jul 15, 2016 •

edited

Loading

levithomason commented Jul 15, 2016 •

edited

Loading

layershifter commented Jul 17, 2016

layershifter commented Jul 17, 2016 •

edited

Loading

layershifter commented Jul 17, 2016

levithomason commented Jul 17, 2016

levithomason commented Jul 17, 2016

levithomason Jul 17, 2016

levithomason Jul 17, 2016

levithomason Jul 17, 2016

levithomason Jul 17, 2016

layershifter commented Jul 17, 2016

levithomason Jul 17, 2016

layershifter commented Jul 17, 2016 •

edited

Loading

layershifter commented Jul 17, 2016

levithomason commented Jul 17, 2016

levithomason commented Jul 17, 2016

layershifter commented Jul 18, 2016

levithomason commented Jul 18, 2016

levithomason commented Jul 18, 2016

layershifter commented Jul 18, 2016

levithomason commented Jul 18, 2016

Update Statistic to v1 API #334

Update Statistic to v1 API #334

Conversation

layershifter commented Jul 15, 2016

API

Types

Statistic

Statistic Group

Content

Value

Label

Variations

Horizontal Statistic

Colored

Inverted

Evenly Divided

Floated

Size

layershifter commented Jul 15, 2016

layershifter commented Jul 15, 2016

codecov-io commented Jul 15, 2016 • edited Loading

Current coverage is 92.47%

levithomason commented Jul 15, 2016 • edited Loading

layershifter commented Jul 17, 2016

layershifter commented Jul 17, 2016 • edited Loading

layershifter commented Jul 17, 2016

levithomason commented Jul 17, 2016

levithomason commented Jul 17, 2016

levithomason Jul 17, 2016

Choose a reason for hiding this comment

levithomason Jul 17, 2016

Choose a reason for hiding this comment

levithomason Jul 17, 2016

Choose a reason for hiding this comment

levithomason Jul 17, 2016

Choose a reason for hiding this comment

layershifter commented Jul 17, 2016

levithomason Jul 17, 2016

Choose a reason for hiding this comment

layershifter commented Jul 17, 2016 • edited Loading

layershifter commented Jul 17, 2016

levithomason commented Jul 17, 2016

levithomason commented Jul 17, 2016

layershifter commented Jul 18, 2016

levithomason commented Jul 18, 2016

levithomason commented Jul 18, 2016

layershifter commented Jul 18, 2016

levithomason commented Jul 18, 2016

codecov-io commented Jul 15, 2016 •

edited

Loading

levithomason commented Jul 15, 2016 •

edited

Loading

layershifter commented Jul 17, 2016 •

edited

Loading

layershifter commented Jul 17, 2016 •

edited

Loading