Leverage rows to update height, not style's height value

[schloerke]

Request from @karangattu

Knowing how many rows exist in the dom structure is much easier to test than setting the height value.

This approach trims lines then adds lines until the scroll height is the client height

[schloerke]

Check failures fixed in #604

[wch]

I'm not sure I understand the purpose of this change -- is it that the existing code has buggy behavior, or is it that the code is confusing?

[schloerke]

I'm not sure I understand the purpose of this change -- is it that the existing code has buggy behavior, or is it that the code is confusing?

Neither.

The current approach is not intuitive to test. Testing for a height of 205px doesn't mentally translate to a height of 10 rows. If the font changes or other css changes occur, the 205 px value will be different. The 10 rows will be consistent regardless of other css values.

---

The new approach may be slightly slower, but the while loops should finish quickly (O(|new height - old height|) ) which is typically difference of 1.

[wch]

I don't love the change... it makes the code slower and more complicated, which in general could be worth it if it improves testability.

But there's a bigger problem which is a showstopper. I just tested it out, and it can cause the browser window to freeze if you do this:

• Type some stuff in the texarea
• Resize the textarea so that it's smaller than the content
• Type

Screen.Recording.2023-07-10.at.9.47.07.AM-half.mp4

I'm guessing the code is getting into an infinite loop. I have a feeling that with this iterative method, you'll have to be very careful about avoiding that situation.

---

Some general thoughts about testing this:

• Can we just test it by, for example typing in some text and checking that the new height is larger than the old height?
• It's in experimental, and we may end up using a completely different input_textarea implementation in the not-too-distant future anyway, so it probably isn't worth investing a lot of energy in testing this.
• Both the old and new implementations don't handle the common case where the textarea is 100% width and then the window is resized to be wider or narrower.

[wch]

Closing based on this comment: #601 (comment)