-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
[Addon: Knobs] Updating text fields is laggy and frequently overwrites keys. #6005
Comments
Is it possible it has something to do with the debounce in KnobStore? |
@CodeByAlex mind taking a look at this? I suspect it's related to #5811 |
@kristremblay, do you know if this was introduced in 5.0.1 or was it present in 5.0.0 |
@CodeByAlex I just started using Storyboard today so I actually don't know. |
@kristremblay, welcome to the community! Would you mind reverting your version of addon/knobs to 5.0 and check. A piece of code that debounces other than what you mentioned above was added to fix the symptoms you are seeing. It would be great if we could pinpoint whether the change had an unintended impact or if the issue was already present. On a separate note, I’m wondering what if the need for the 50ms Debounce in the store is still needed. |
Thanks! This is a great product you guys have made and it's certainly making my life easier :) It looks like it is working properly in 5.0.0. I saw the debounce and assumed that had something to do with it. |
That’s good to know. @shilman I’ll take a look into this further. |
@CodeByAlex @ndelangen How about posting the knob change when the user hits enter or focus leaves the text field, rather than on every key press? |
I have the exact same issue. Was happening in 5.0 and upgraded to 5.0.1 and the issue seemed to be fixed slightly but I still have to type slower than usual without characters being dropped. I think posting the knob update on blur of the knob would be better IMO |
@shilman it looks like the Debounce that was added to reduce lag on the Angular side caused the choppy text entry/dropping of characters that @kristremblay was mentioning in Vue. We will have to back that out and put in a different solution. The frameworks seem to react differently to this knobs rendering logic in general. |
I can provide links to anyone who picks up this issue with examples that we are experiencing this on both 5.0.0 and 5.0.1 so I'm not so sure its a direct result of the 5.0.1 changes, I just can't share the links publically since its a proprietary storybook. If you DM me (shared them with @shilman on discord as well) I can send you links of the issue happening on both versions. |
@shilman @kylepeeler @kristremblay
So as of the 5.0.1 update, the logic in the debounce function consisted of:
After adding the following back in the onchanged function before the debounce, the issue in vue is resolved:
To better handle the debouncing and force rendering, instead of waiting for a user to click out of the field, we should wait for a longer pause before rendering. In the 5.0.1 version, its waiting 150ms which is a little faster than the average typing speed. I suggest we wait closer to 400ms and then just have it render when the user has completed their thought. What do you guys think? |
I think that's a good solution. Let's see how it works in the next commit :) |
PR for this change can be found here: #6022 |
Crikey!! I just released https://github.com/storybooks/storybook/releases/tag/v5.1.0-alpha.5 containing PR #6022 that references this issue. Upgrade today to try it out! Because it's a pre-release you can find it on the Closing this issue. Please re-open if you think there's still more to do. |
@shilman Perhaps I'm doing something wrong, but running
|
@kristremblay Hmm try clearing out |
@shilman No luck, same errors. |
Have you tried deleting your package.lock file?
…On Tue, Mar 12, 2019 at 8:38 PM Kris Tremblay ***@***.***> wrote:
@shilman <https://github.com/shilman> No luck, same errors.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6005 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AcYoBiAfAV30ulIrrAmzekbJ5X8EfScMks5vWEhsgaJpZM4bnfUw>
.
|
Yup, still busted. |
I'm going to go try it on my mbp and see if it's having the same issue. |
@CodeByAlex @shilman Nope, same thing on both machines, went through deleting |
Hmm not sure what to say. It's working fine in our demo Vue storybook and I can't think of any changes in 5.1 that would cause this error and nothing about your story above jumps out at me. Here are some things I'd try:
Thanks for your patience on this! |
I’ll have to double check when I get home in a few hours. Reverting to 5.0.x works (with 5.0.0 being the best one as far as the debounce issue is concerned). I really appreciate the responsiveness, I just wanna see it work :D |
Same error after upgrade to alpha5. Vue project created with vue cli and vue cli storybook folder node_modules was deleted. In package.json: Reverting to 5.0.0 works. 5.0.1 was bad update to knobs) |
Jeepers creepers!! I just released https://github.com/storybooks/storybook/releases/tag/v5.0.2 containing PR #6022 that references this issue. Upgrade today to try it out! |
since the 4.2 alpha introduced my changes to the storybook vue, debouncing the knobs should not be necessary at all for vue, since we properly change the props of the components in a reactive "vue way". |
General note: Debounce can be necessary on systems with poorly functioning keyboards (not that macbooks have ever had anything other than perfect keyboards). However, debounce should, by default, only take effect for quickly repeated instances of the same character. |
Describe the bug
Editing text fields in knobs seemingly lags behind key presses, and deletes any characters after one of such instances. It's like the state is updating in a convoluted manner, and previously entered characters are overwritten. For example, quickly typing "Kris Tremblay" at the same speed I'd normally type yielded "Krisblay".
To Reproduce
Steps to reproduce the behavior:
Expected behavior
The field should act as a normal input, and changes should be reflected in the component.
Code snippets
The following is my test story for an ActionButton component.
System:
The text was updated successfully, but these errors were encountered: