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

fix: make NumberCell for Vanilla Renderer show 0 #1658

Merged
merged 1 commit into from
Dec 2, 2020
Merged

fix: make NumberCell for Vanilla Renderer show 0 #1658

merged 1 commit into from
Dec 2, 2020

Conversation

JovanShandro
Copy link
Contributor

When using a default value of 0 for a number cell with vanilla renderers, the input is shown with an empty string. This is a quick fix for it.

@sdirix
Copy link
Member

sdirix commented Dec 1, 2020

Thank you very much for the contribution! Could you also add a testcase?

Copy link
Member

@sdirix sdirix left a comment

Choose a reason for hiding this comment

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

A smaller fix could be to replace data || '' with data ?? '', right? If you agree please adapt this PR.

return (
<input
type='number'
step='0.1'
value={data || ''}
value={value}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
value={value}
value={data ?? ''}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, thanks! I am making the change now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also I would love to add a testcase, but for the moment I am not familiar with how enzyme works, or testing for react components in general, so it might take me some time 😅

@sdirix sdirix added this to the 2.5.0 milestone Dec 1, 2020
@coveralls
Copy link

coveralls commented Dec 1, 2020

Coverage Status

Coverage remained the same at 88.445% when pulling 4a462c9 on JovanShandro:master into a51e373 on eclipsesource:master.

The React Vanilla NumberCell now correctly displays the number 0
instead of an empty string.

Co-authored-by: Stefan Dirix <sdirix@eclipsesource.com>
@sdirix sdirix merged commit dcb224d into eclipsesource:master Dec 2, 2020
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.

3 participants