Skip to content

feat(Gauge): add new component - #466

Merged
igorarkhipenko merged 11 commits into
masterfrom
223-gauge-component
Mar 19, 2021
Merged

feat(Gauge): add new component#466
igorarkhipenko merged 11 commits into
masterfrom
223-gauge-component

Conversation

@igorarkhipenko

@igorarkhipenko igorarkhipenko commented Mar 16, 2021

Copy link
Copy Markdown
Contributor

Checklist

  • The implementation has been manually tested and complies with Textkernel browser support guidelines
  • The implementation complies with accessibility standards.
  • The component has a displayName defined.
  • The component comes with a detailed PropTypes (and defaultProps) definition.
  • Component PropTypes are sufficiently described / documented.
  • There is a story in Storybook.

Comment thread src/components/Gauge/Gauge.tsx Outdated
Comment thread src/components/Gauge/Gauge.tsx Outdated
Comment thread src/components/Gauge/Gauge.tsx
Comment thread src/components/Gauge/Gauge.tsx
Comment thread src/components/Gauge/Gauge.tsx Outdated
) : (
<span {...elem('content', props)}>
{children}
{metric ? <span {...elem('metric', props)}>{metric}</span> : null}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If children and metric are ReactNodes, then there is a good chance that wrapping them in span will create incorrect HTML.

Comment thread stories/Gauge.tsx
<Gauge
context={select('Context', CONTEXTS, CONTEXTS[1])}
percentage={number('Percentage', 33)}
note={isContentLoading ? '—' : 'Low'}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It took me sometime to test that it will work with tooltips and other complex things. Maybe you could add one such example

Comment thread src/components/Gauge/Gauge.scss
Comment thread stories/Gauge.tsx
@carlobernardini

Copy link
Copy Markdown
Contributor

@igorarkhipenko It seems like the CircleCI checks did not run on this PR, maybe it was opened before we moved to CircleCI? Maybe you can rebase your branch and force-push.

@igorarkhipenko
igorarkhipenko merged commit 557b979 into master Mar 19, 2021
@igorarkhipenko
igorarkhipenko deleted the 223-gauge-component branch March 19, 2021 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants