feat: ui.text_area #652

AkshatJawne · 2024-07-22T20:53:32Z

Closes #649

Additional Info:

Did not include validate prop based off this comment on Text Field PR: docs: text field #357 (comment)

Some of the snippets I had tested with:

from deephaven import ui

def test_on_change(new_value):
    print(f"Text changed to {new_value}")

t1 = ui.text_area()
t2 = ui.text_area(on_change=test_on_change)
t3 = ui.text_area(label="Test Label", label_position='side', flex_grow=1.0, flex_shrink=0.75)
t4 = ui.text_area(label="Test Label 2", is_required=True)
t5 = ui.text_area(is_disabled=True, is_hidden=False)
t6 = ui.text_area(is_hidden=True)
t7 = ui.text_area(auto_focus=True)

mofojed

Include some examples that you tested with. Looking good

mofojed · 2024-07-24T14:33:04Z

plugins/ui/src/js/src/elements/TextArea.tsx

+  const [value, setValue] = useState(propValue ?? defaultValue);
+  const [pending, setPending] = useState(false);
+  const prevPropValue = usePrevious(propValue);
+
+  // Update local value to new propValue if the server sent a new propValue and no user changes have been queued
+  if (
+    propValue !== prevPropValue &&
+    propValue !== value &&
+    propValue !== undefined &&
+    !pending
+  ) {
+    setValue(propValue);
+  }
+
+  const propDebouncedOnChange = useCallback(
+    async (newValue: string) => {
+      try {
+        await propOnChange(newValue);
+      } catch (e) {
+        log.warn('Error returned from onChange', e);
+      }
+      setPending(false);
+    },
+    [propOnChange]
+  );
+
+  const debouncedOnChange = useDebouncedCallback(
+    propDebouncedOnChange,
+    VALUE_CHANGE_DEBOUNCE
+  );
+
+  const onChange = useCallback(
+    (newValue: string) => {
+      setPending(true);
+      debouncedOnChange(newValue);
+      setValue(newValue);
+    },
+    [debouncedOnChange]
+  );


We should be able to extract this into a hook and share it with TextField. Something like const [value, onChange] = useDebouncedValue<T = string>(propValue, defaultValue, propOnChange)

mofojed · 2024-07-24T14:35:41Z

plugins/ui/src/js/src/elements/TextArea.tsx

+    VALUE_CHANGE_DEBOUNCE
+  );
+
+  const onChange = useCallback(


Side note about onChange - we should only have it set to a defined value if the passed in propOnChange is defined. In some cases, simply passing in a function will change the behaviour of the component, so we don't want to always pass in a function if the propOnChange function was not provided.

So in this case it'd be something like:

const onChangeCallback = useCallback( ... ); const onChange = propOnChange ? onChangeCallback : undefined;

See useFormEventCallback.ts as another example (and it's usage in Form.tsx).

mofojed · 2024-07-24T14:43:04Z

plugins/ui/src/deephaven/ui/components/text_area.py

+
+
+def text_area(
+    icon: Element | None = None,


For convenience, we can probably allow an icon string here as well, and automatically convert it to a ui.icon element if it's provided.

Do we want to do this though, seems inconsistent since we don't do it tab.py , text_field.py, list_action_group.py with summary_icon?

bmingles · 2024-07-26T15:53:12Z

plugins/ui/src/js/src/elements/hooks/useDebouncedOnChange.ts

+
+const VALUE_CHANGE_DEBOUNCE = 250;
+
+function useDebouncedOnChange<T = string>(


This hook should probably be implemented in @deephaven/react-hooks. Doesn't seem to have anything plugins specific unless I'm missing someting.

@AkshatJawne Does this add any functionality that useDebouncedCallback doesn't already provide?

nvm, I see what it is doing.

I can look into implementing on the DHC side, thought it would be more appropriate here, since Bender had referenced the useFormEventCallback.ts, and that lives in the plugins repo.

Where is useFormEventCallback referenced?

#652 (comment)

I think that's just an example of a hook that properly handles the undefined callback scenario and not necessarily tying it to plugins repo.

Wouldn't be opposed to it, if @mofojed has no issues with it, I can do it -- just holds up the process a little more, have to create a separate web-client-ui PR, wait for that to merge, and then wait for the version bump and then this will be able to be merged, right?

No need to move it to @deephaven/react-hooks - we can keep it in mind but there's no pressing need to, we don't need to wait for a bump/publish cycle to use it here, just add it here.

bmingles · 2024-07-26T16:00:17Z

plugins/ui/src/js/src/elements/hooks/useDebouncedOnChange.ts

+
+function useDebouncedOnChange<T = string>(
+  propValue: T | undefined,
+  defaultValue: T,


I don't think the defaultValue prop makes sense inside of this hook. I would just support value and onChange inputs and coalesce the value ?? defaultValue in the calling component.

Good shout, yeah I think this makes sense, will make change.

There is a chance you can use useDebouncedValue and don't need a new hook at all.

Gonna try rn

nvm, I don't think that allows the debouncing in the change handler passed into the component

Although, just on first sight, I will need onChange for the textField, which the useDebouncedValue doesn't give access to

Yeah, disregard the useDebouncedValue idea :)

plugins/ui/src/deephaven/ui/types/types.py

plugins/ui/src/js/src/elements/TextArea.tsx

plugins/ui/src/deephaven/ui/types/types.py

plugins/ui/src/js/src/elements/TextArea.tsx

plugins/ui/src/js/src/elements/hooks/useDebouncedOnChange.ts

mofojed · 2024-07-29T15:32:49Z

plugins/ui/src/js/src/elements/TextArea.tsx

+  const icon =
+    typeof propIcon === 'string' ? (
+      <Icon>
+        <FontAwesomeIcon icon={getIcon(`deephaven.ui.icons.${propIcon}`)} />
+      </Icon>
+    ) : (
+      propIcon
+    );


I would prefer this on the Python side. Just change icon=icon on text_area.py#187 to icon=ui.icon(icon) if type(icon) == 'str' else icon and it should be good.
It's better practice to do it Python side when possible as we may have default props set in the Python code.

Been trying to work with this, causing an error on rendering, trying to debug:

Closes #678 Assuming the changes proposed in #652

feat: ui.text_area

ab1adfb

AkshatJawne requested a review from mofojed July 22, 2024 20:53

AkshatJawne self-assigned this Jul 22, 2024

small cleanup

620bf50

mofojed requested changes Jul 24, 2024

View reviewed changes

address changes

c6563a7

AkshatJawne requested a review from mofojed July 25, 2024 05:53

fix error

0f0f3ca

AkshatJawne mentioned this pull request Jul 26, 2024

feat: Javascript DatePicker Implementation #667

Merged

bmingles reviewed Jul 26, 2024

View reviewed changes

AkshatJawne added 3 commits July 26, 2024 10:07

remove defaultVal prop

c1a9deb

adjust icon prop to also be able to take str

ed6a525

add icon type alias

49c8d80

AkshatJawne requested a review from bmingles July 29, 2024 13:06

bmingles reviewed Jul 29, 2024

View reviewed changes

plugins/ui/src/deephaven/ui/types/types.py Show resolved Hide resolved

bmingles reviewed Jul 29, 2024

View reviewed changes

plugins/ui/src/js/src/elements/TextArea.tsx Outdated Show resolved Hide resolved

small cleanup

b3461d2

AkshatJawne requested a review from bmingles July 29, 2024 15:14

remove comment

d76de23

mofojed requested changes Jul 29, 2024

View reviewed changes

make changes based on review

fffacab

AkshatJawne requested a review from mofojed July 30, 2024 00:51

mofojed approved these changes Jul 30, 2024

View reviewed changes

AkshatJawne mentioned this pull request Jul 30, 2024

docs: ui.text_area #683

Merged

AkshatJawne merged commit 5fb24bc into deephaven:main Jul 30, 2024
15 checks passed

AkshatJawne added a commit that referenced this pull request Aug 7, 2024

docs: ui.text_area (#683)

4df5ba3

Closes #678 Assuming the changes proposed in #652

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: ui.text_area #652

feat: ui.text_area #652

AkshatJawne commented Jul 22, 2024 •

edited

Loading

mofojed left a comment

mofojed Jul 24, 2024

mofojed Jul 24, 2024

mofojed Jul 24, 2024

AkshatJawne Jul 25, 2024

bmingles Jul 26, 2024

bmingles Jul 26, 2024

bmingles Jul 26, 2024

AkshatJawne Jul 26, 2024

bmingles Jul 26, 2024

AkshatJawne Jul 26, 2024

bmingles Jul 26, 2024

AkshatJawne Jul 26, 2024

mofojed Jul 29, 2024

bmingles Jul 26, 2024

AkshatJawne Jul 26, 2024

bmingles Jul 26, 2024

AkshatJawne Jul 26, 2024

bmingles Jul 26, 2024

AkshatJawne Jul 26, 2024

bmingles Jul 26, 2024 •

edited

Loading

mofojed Jul 29, 2024

AkshatJawne Jul 29, 2024 •

edited

Loading


		const VALUE_CHANGE_DEBOUNCE = 250;

		function useDebouncedOnChange<T = string>(

feat: ui.text_area #652

feat: ui.text_area #652

Conversation

AkshatJawne commented Jul 22, 2024 • edited Loading

mofojed left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmingles Jul 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AkshatJawne Jul 29, 2024 • edited Loading

Choose a reason for hiding this comment

AkshatJawne commented Jul 22, 2024 •

edited

Loading

bmingles Jul 26, 2024 •

edited

Loading

AkshatJawne Jul 29, 2024 •

edited

Loading