-
-
Notifications
You must be signed in to change notification settings - Fork 159
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
Invalid unit type when insert number in number control #291
Comments
@likern thanks for reporting this. I can see why this is an issue. I suppose that on blur or similar could be used to defer the update until the user is finished. If you want to create a pr with your suggested solution that would be great. If not that's no problem, I'll make sure to include a fix before 6.0. |
@dannyhw Unfortunately, I'm very busy with my duties. I can only report issues I've found while integrating StoryBook. Not able to contribute to this project. |
@likern no problem, thanks for reporting issues, that's already a big help 🙏 |
@dannyhw Would like to contribute to this one. I found that if onBlur is used, when the user finishes changing the number input and press the preview or navigator tab at the bottom or pressing the component preview section, the onBlur handler will not be called. (I am testing on an iOS simulator) Also, I found that the type of argument that the onChange handler receives is of type string after the user changes the number input. Would it be better to ensure the argument type to always be number instead of sometimes string and sometimes number? I think it would mitigate this issue and some other issues like |
@raychanks hey thats great, all contributions welcome. I've tried before to resolve this however I didn't find a simple solution. On the one hand I understand that its unfortunate when the app can crash from the wrong input however I think that it should be possible for the user to test these kinds of cases with storybook and im not sure we should sanitise input. You could make the argument that the users components should be resilient to undefined values but I can understand it from both sides. Doing a onblur or on submit seems like a good way around the current functionality however as it currently works when you press the preview the addons panel is immediately unmounted and any on blur is not executed which leaves you with and awkward user experience. Using onsubmit and blur together could provide something workable but getting the blur to trigger might require some wrangling on the ondevice code. Feel free to hack away at it, I would love to see what solution you come up with and I'm open to different approaches. |
@dannyhw hacked a version to defer the change by handling it on blur and submit, please help reviewing it when you have time. On the other hand, I would like to continue the discussion about the data type issue. I am still quite new to this project and not so familiar with how the users normally use storybook. I agree that storybook should allow users to test different input types to make sure the components they built are resilient enough to handle different cases. However, I think these kind of tests should be performed by the user explicitly, for example changing the control type in a story from number to text to explicitly test for the behavior of passing in a string. At the time being, the NumberType input will implicitly changes the data type from number to string after modification, and the data type will keep on being string in subsequent changes, if the user actually wants the input type to be number at all times, we seem to not have a way to cater for it. |
@raychanks thats an interesting point actually I guess you're right. I'm open to enforcing the number values. Thanks a lot for opening the PR for this, I'll take a look soon :). |
@dannyhw Oh if that's the case, I think the changes can be simplified and do not need to rely on the submit and blur handlers. I will update the code later tonight and mark the pull request as WIP for now. Thanks for your effort too :) |
This reverts commit 1f97003.
This reverts commit 1f97003.
This reverts commit 1f97003.
This reverts commit 1f97003.
This reverts commit 1f97003.
Describe the bug
When I use control type
number
for propertypreview rerenders immediately while I'm typing new value.
That leads to invalid empty string being passed to as number value (when I delete old number and try to insert new number).
Expected behavior
Don't rerender immediately, only when Submit button on keyboard was pressed.
Screenshots
System:
The text was updated successfully, but these errors were encountered: