Skip to content

Conversation

@ffMathy
Copy link

@ffMathy ffMathy commented Feb 11, 2020

This will make it possible to use my own progress bar if needed.

Also fixed typings of style prop (which in turn removed linter warning), and also improved the type of fetch to be more precise.

This will make it possible to use my own progress bar if needed.
@ffMathy ffMathy requested a review from jvhoven February 11, 2020 08:47
@ffMathy ffMathy changed the title Provide custom way of rendering progress bar Provide custom way of rendering progress bar + typing fixes Feb 11, 2020
@codecov
Copy link

codecov bot commented Feb 11, 2020

Codecov Report

Merging #6 into master will decrease coverage by 4.83%.
The diff coverage is n/a.

Impacted file tree graph

@@ Coverage Diff @@ ## master #6 +/- ## =========================================== - Coverage 100.00% 95.16% -4.84%  =========================================== Files 1 1 Lines 48 62 +14 Branches 3 14 +11 =========================================== + Hits 48 59 +11  - Misses 0 3 +3 
Impacted Files Coverage Δ
src/index.tsx 95.16% <0.00%> (-4.84%) ⬇️

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 5647e63...fab3212. Read the comment docs.

@ffMathy
Copy link
Author

ffMathy commented Feb 11, 2020

Codecov fails, but I think it is still reasonable. What do you think?

@MrHus
Copy link
Contributor

MrHus commented Feb 11, 2020

Codecov fails, but I think it is still reasonable. What do you think?

Good idea to support a render function.

Two things:

  1. The test coverage should be 100% and test the render function.

  2. We should add a children render prop as well

interface Props {
style?: Record<string, any>;
style?: React.CSSProperties;
render?: (mode: Mode) => JSX.Element;
Copy link
Contributor

Choose a reason for hiding this comment

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

React.ReactNode should be the return type of the render.

@ffMathy
Copy link
Author

ffMathy commented Feb 11, 2020

Can you elaborate what you mean by the children prop?

@MrHus
Copy link
Contributor

MrHus commented Feb 12, 2020

Can you elaborate what you mean by the children prop?

@ffMathy

That we support both: render and children render function props:

<ProgressBar render={mode => <span>{mode}</span}/>
<ProgressBar>{ mode => <span>{mode}</span}</ProgressBar>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants