Doist / reactist

Open source React components made with ❤️ by Doist
http://doist.github.io/reactist
MIT License
248 stars 22 forks source link

A controlled `TextArea` with `autoExpand` doesn't resize correctly on initial render #771

Closed pawelgrimm closed 1 year ago

pawelgrimm commented 1 year ago

🐛 Bug report

Current behavior

When you use TextArea in controlled-mode (i.e. passing in a value prop) and have turned on autoExpand, the textarea will not be sized correctly on the initial render. However, it will resize once the user edits the value. This is especially noticeable if you set rows={1} and have an initial value that doesn't fit on one line.

This happens because we only resize the text area when the input event is fired, not whenever the value prop changes.

image

Steps to reproduce the bug

Edit the text-area.stories.tsx story for AutoExpandStory to this:

export function AutoExpandStory(props: PartialProps<typeof TextArea>) {
    const [value, setValue] = useState(
        "This is some text that takes up multiple lines. Now, notice that at first, it is cut off a bit until you actually edit it.",
    )

    return (
        <Stack space="xxlarge" dividers="secondary" maxWidth="medium">
            <TextArea
                {...props}
                label="What do you want to accomplish?"
                secondaryLabel="auto-expand enabled"
                hint="Write as much or as little as you want. The input area will auto-expand to fit what you've typed."
                value={value}
                onChange={(event) => setValue(event.target.value)}
                rows={1}
                autoExpand
            />
            <TextArea
                {...props}
                label="What do you want to accomplish?"
                secondaryLabel="No auto-expand"
                hint="This one will not auto-expand."
                autoExpand={false}
            />
        </Stack>
    )
}

Expected behavior

The autoExpand mechanism should kick in on the initial render.

Possible solutions

Update the useEffect in TextArea to apply the value prop to containerElement.dataset.replicatedValue. Note that this means we'd be setting containerElement.dataset.replicatedValue twice on each edit:

  1. Once via the event listener
  2. Another time when the value props is changed and causes a re-render
    React.useEffect(
        function setupAutoExpand() {
            const containerElement = containerRef.current
            function handleInput(event: Event) {
                if (containerElement) {
                    containerElement.dataset.replicatedValue = (event.currentTarget as HTMLTextAreaElement).value
                }
            }

            const textAreaElement = internalRef.current
            if (!textAreaElement || !autoExpand) return undefined

            // Set up the initial height of the textarea (if we are operating in controlled mode)
            if (containerElement && value) {
                containerElement.dataset.replicatedValue = value
            }

            textAreaElement.addEventListener('input', handleInput)
            return () => textAreaElement.removeEventListener('input', handleInput)
        },
        [value, autoExpand],
    )

There are a few ways to work around this, but I don't have the appetite to propose a complete solution at this time. Additionally, the type of the value prop is incorrect, as it flows down from the HTMLInputElement, which accepts a more diverse set of values than HTMLTextAreaElement, which only accepts string.

Environment

gnapse commented 1 year ago

Nice catch. This happens not only on a controlled component, but anytime that the text area has an initial value that does not fit in its initial size (which can happen if we pass defaultValue="very long text" as well).

I got a potential fix in #772 that does not do anything on value changes, but only calling the code that enables autoExpand once when the auto-expand effect runs for the first time. I'm not sure if I'm missing something, but I do not see the reason to make the effect re-run when the value changes. Let me know what you think.