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

Do not render empty mark labels #340

Merged
merged 1 commit into from
Nov 3, 2017

Conversation

uwolfer
Copy link
Contributor

@uwolfer uwolfer commented Oct 20, 2017

Use-case: I have a slider bar for percent values. I use marks for
every allowed value (e.g. values from 2 to 5 are not allowed).
Displaying every mark label would flood the UI. So there are mark labels
displayed only for every 10th step, which also works fine without this
change (using: {10: '10%', 11: ''}).
This change just optimizes that there are no empty DOM nodes rendered.

Use-case: I have a slider bar for percent values. I use marks for
every allowed value (e.g. values from 2 to 5 are not allowed).
Displaying every mark label would flood the UI. So there are mark labels
displayed only for every 10th step, which also works fine without this
change (using: {10: '10%', 11: ''}).
This change just optimizes that there are no empty DOM nodes rendered.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 61.739% when pulling 0f74224 on uwolfer:empty-mark-labels into 76d4cf3 on react-component:master.

@paranoidjk
Copy link
Member

Sorry, could you explain what problems does render empty mark labels will cause ?

IMO, empty string actually is a valid React Element, if we add too many such check in a React Component, it will be redundancy.

@uwolfer
Copy link
Contributor Author

uwolfer commented Oct 26, 2017

It's not a bugfix. It's just an optimization. This change just prevents rendering useless empty dom nodes.

@paranoidjk paranoidjk merged commit 62d2375 into react-component:master Nov 3, 2017
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