-
-
Notifications
You must be signed in to change notification settings - Fork 42
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
[TextareaAutosize] Fix resizing instability #189
[TextareaAutosize] Fix resizing instability #189
Conversation
I'm having trouble coming up with a test for this. @oliviertassinari, since you added the "PR: needs test" label, could you assist me with it? |
We are not going to release any new versions from this repo anytime soon (not unless we have a consistent API across existing components). We decided not to encourage contributions to the Base UI in the Core repo either (so we don't end up managing two diverging versions), but since you already took the effort to fix the bug, we can release it from the Core repo. I understand that it's not a perfect situation and we're doing what we can to have just one copy of the source code as soon as possible. |
Since there is no one copy of the source code yet, I think we should fix it here in this repository too, or it will become out of sync. Once it's good here, I'll make a PR in the Core repository too. I think we should point it to the |
@DiegoAndai, @mnajdova - what would be the best way to release this from the Core repo? |
looking forward to seeing this fixed |
Auto-sizing can be handled with Here's a demo. |
As discussed today in the core team meeting: We can release whenever we have something to release. The process is to merge the change to |
Created PR in Core repo: mui/material-ui#41638. |
@mui/base-ui I believe we should consider merging this. See the rationale in #168 (comment). The corresponding PR in the core repository (mui/material-ui#41638) has been merged, for existing Regarding the test, I've implemented an E2E test in the Core repository. However, we currently lack the setup for E2E tests here. Should I proceed with setting up an E2E test suite, or should we consider merging this PR without a test? |
All right, we can merge it here as the future of this component is not clear yet. As for E2E tests, could you set them up in a separate PR? |
I just saw it's being done in #395. |
All right, let's add the test once #395 is merged and we'll be able to close it. |
@michaldudak Added test. Ready for merge. |
Fixes #168
Fixes #160
Thanks to @hyorimitsu for pinpointing the cause of the issue, as mentioned in #168 (comment).
The solution involves storing the height in a ref and updating it only when it differs from the previous value. Previously, we set the height everytime even if it remained unchanged, causing the glitch.
Before: https://codesandbox.io/p/sandbox/old-wave-py2jkq
After: https://codesandbox.io/p/sandbox/admiring-dan-9kqmtk
If this looks good, should I open a PR in the Material UI repository as well? This fix addresses a regression, and I'm uncertain about when it will be released from this repository.