Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

breaking(Statistic): refactor Statistic to use factories, update content props #2143

Merged
merged 2 commits into from
Oct 10, 2017

Conversation

layershifter
Copy link
Member

@layershifter layershifter commented Sep 29, 2017

Why?


Statistic.Label, Statistic.Value

Before After
<Statistic.Label label='foo' /> <Statistic.Label content='foo' />
<Statistic.Label label={<div>foo</div>} /> <Statistic.Label content={<div>foo</div>} />
<Statistic.Value value='foo' /> <Statistic.Value content='foo' />
<Statistic.Value value={<div>foo</div>} /> <Statistic.Value content={<div>foo</div>} />

Statistic

Before

<Statistic
  label={<div>label</div>}
  value={<div>value</div>}
/>

After

<Statistic
  label={{ content: <div>label</div> }}
  value={{ content: <div>value</div> }}
/>
// or
<Statistic
  label={<Statistic.Label>label</Statistic.Label>}
  value={<Statistic.Value>value</Statistic.Value>}
/>

Statistic.Group

Before

const items = [
  { label: 'foo', value: 'bar' },
  { label: 'baz', value: 'qux' },
]
<Statistic.Group items={items} />

After

key is required now.

const items = [
  { key: 'foo', label: 'foo', value: 'bar' },
  { key: 'baz', label: 'baz', value: 'qux' },
]
<Statistic.Group items={items} />

@codecov-io
Copy link

Codecov Report

Merging #2143 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2143      +/-   ##
==========================================
+ Coverage   99.76%   99.76%   +<.01%     
==========================================
  Files         150      150              
  Lines        2597     2600       +3     
==========================================
+ Hits         2591     2594       +3     
  Misses          6        6
Impacted Files Coverage Δ
src/views/Statistic/Statistic.js 100% <100%> (ø) ⬆️
src/views/Statistic/StatisticLabel.js 100% <100%> (ø) ⬆️
src/views/Statistic/StatisticGroup.js 100% <100%> (ø) ⬆️
src/views/Statistic/StatisticValue.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8ecb71d...c4ee75e. Read the comment docs.

@levithomason
Copy link
Member

One question, why is this syntax not supported anymore?

<Statistic
  label={<div>label</div>}
  value={<div>value</div>}
/>

Factories support passing elements.

@layershifter
Copy link
Member Author

layershifter commented Oct 1, 2017

Statistic now uses factories to create Statistic.Label and Statistic.Value, so this example will produce following markup:

<div class="ui statistic">
  <div>value</div>
  <div>label</div>
</div>

It will don't contain valid classNames.

@levithomason
Copy link
Member

levithomason commented Oct 10, 2017

Heads up, in the future, I'd like to make factories ensure required classNames are present 👍 Thanks for the work here!

@levithomason
Copy link
Member

Released in semantic-ui-react@0.75.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants