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

<Textarea> Update #72

Open
daniel-cy-lu opened this issue Sep 19, 2022 · 0 comments
Open

<Textarea> Update #72

daniel-cy-lu opened this issue Sep 19, 2022 · 0 comments

Comments

@daniel-cy-lu
Copy link
Contributor

daniel-cy-lu commented Sep 19, 2022

Detailed Description

Purpose: Update useEffect in <Textarea> to make rendering work when clearing contents using useState.
Issue: <Textarea> won't render properly when using useState to clear content.

See Ciaran's Explaination:

Textarea is not re-rendering because it's state is not changing. The value prop is only used to set the initial state of internalValue .
To fix, we can add value prop to the dependency array of the useEffect on line 130. This makes the function run every time the value prop changes, not just on component mounting, which is what an empty dependency array does [ ]
We also use the already existing functionality of applyChanges so nothing should break and it can reconcile any changes needed internally.

useEffect(() => {
    value && applyChanges(value);
  }, [value]);

Now if you try this it's still broken... but our state is being passed correctly like you said. Well, empty string '' is a falsey JS value so the shorthand of value && applyChanges(value) means applyChanges is never run. But empty string is a valid value for our textarea. So we can fix the conditional like so:

useEffect(() => {
  (value || value === '') && applyChanges(value);
}, [value]);

Possible Implementation

Locate <Textarea> in src/form and update line 127-130 to:

useEffect(() => {
    (value || value === '') && applyChanges(value);
  }, [value]);

Or Justin suggests to:

useEffect(() => {
    (value !== internalValue) && applyChanges(value);
  }, [value]);

Reason of this slight change is so that we don't need to keep adding conditions in the future.
Both cases work for the filter modal textarea.

@daniel-cy-lu daniel-cy-lu changed the title Beta-Feedback <Textarea> Update Sep 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant