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: do not grow autoresized text area on first input #216

Merged
merged 4 commits into from
Feb 8, 2023

Conversation

barmac
Copy link
Member

@barmac barmac commented Feb 7, 2023

No description provided.

@barmac barmac requested a review from nikku February 7, 2023 15:56
@bpmn-io-tasks bpmn-io-tasks bot added the needs review Review pending label Feb 7, 2023
@nikku
Copy link
Member

nikku commented Feb 7, 2023

Thanks for looking into this. I don't understand why the scrollHeight check should be necessary. We enforce a minimum height via CSS. Removing it (b1b8a10) the test still passes.

There is a couple of nuances that we need to be aware on non MacOS systems regarding scrollbars. Scroll height will always be < then offsetHeight once resized, hence we must make the actual height magic 2px bigger than scroll height:

capture QmX6JA_optimized

If we don't do that then a pesky scrollbar appears every time we auto-resize.

Finally, revisiting this I find that our cap of 150px is an arbitrary one, and dangerous, too. We'd need to properly handle manual user resize in order to turn that into a meaningful thing. I've removed it via 29e9612.

Please check if rows=1 was the actual fix for the issue. Consider squashing my PRs then.

Copy link
Member

@nikku nikku left a comment

Choose a reason for hiding this comment

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

I think this can be simplified, and + 2px must be kept: #216 (comment).

@bpmn-io-tasks bpmn-io-tasks bot added in progress Currently worked on and removed needs review Review pending labels Feb 7, 2023
@barmac
Copy link
Member Author

barmac commented Feb 8, 2023

It still grows on MacOS / Chrome. It looks like the +2px causes this:

Screen.Recording.2023-02-08.at.08.57.48.mov

@barmac
Copy link
Member Author

barmac commented Feb 8, 2023

Please check if rows=1 was the actual fix for the issue. Consider squashing my PRs then.

rows=2 makes it grow to 2 lines height on my machine.

@barmac barmac merged commit f0fbef0 into main Feb 8, 2023
@barmac barmac deleted the autoresize-problem branch February 8, 2023 08:33
@bpmn-io-tasks bpmn-io-tasks bot removed the in progress Currently worked on label Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants