[WIP] [EuiInlineEdit] Allow `EuiInlineEdit` to be used as a controlled component by breehall · Pull Request #7117 · elastic/eui

breehall · 2023-08-23T18:38:29Z

🛑 WIP - DO NOT MERGE

Summary

Allows EuiInlineEdit to be used as a controlled component with the addition of three new props
More to come (this is a WIP)

Feature Checklist

Create three new props
1. value - controlled value for consumers
2. onChange - onChange callback that returns the HTML event
3. onCancel - onCancel callback that returns the previous value
Create exclusive union between defaultValue and {value, onChange}
Create useMemo to return valueToUse
1. Should return which value should be used in edit mode and read mode elements.
  1. If ( value ) - use it | Else Fall back to readModeValue / editModeValue based on isEditing
2. Replace instances of readModeValue & editModeValue with valueToUse
Pass onChange from props to edit mode form element (ln. 238)

QA

Remove or strikethrough items that do not apply to your PR.

General checklist

…as a controlled component. - Added the value and onChange props to be used when inline edit is a controlled component - Separated defaultValue and controlled props (value & onChange) into an exclusive union to ensure inline edit cannot be used in both states at once

- Add the onCancel callback prop to allow consumers to fallback to the previous inline edit value upon cancel - Create a DRY helper to switch between readModeValue, editModeValue, and value appropriately

…Title

elasticmachine · 2023-08-23T18:47:46Z

💔 Build Failed

Buildkite Build
Commit: 51ad584

Failed CI Steps

🔨 EUI pull request test

History

breehall · 2023-08-23T18:49:00Z

src/components/inline_edit/inline_edit_form.tsx


  const cancelInlineEdit = () => {
    setEditModeValue(readModeValue);
+    onCancel && onCancel(readModeValue);


I am using readModeValue instead of valueToUse here because is contains the prior text value

kibanamachine · 2023-08-23T19:04:40Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_7117/

cee-chen · 2023-08-23T23:24:58Z

src-docs/src/views/inline_edit/inline_edit_title.tsx

+        value={inlineEditValue}
+        onChange={(e) => {
+          setInlineEditValue(e.target.value);
+        }}
+        onCancel={(previousValue) => {
+          setInlineEditValue(previousValue);
+        }}


👌💋 B-E-A-utiful developer experience!

cee-chen · 2023-08-23T23:29:05Z

src/components/inline_edit/inline_edit_form.tsx

    // If an onSave callback is present, and returns false, stay in edit mode
    if (onSave) {
-      const onSaveReturn = onSave(editModeValue);
+      const onSaveReturn = onSave(valueToUse);


We'll probably also want to test that onSave works with a controlled value as well. How you prefer to test that is up to you; whether via docs, Jest test, or Storybook

cee-chen · 2023-08-23T23:31:22Z

src/components/inline_edit/inline_edit_form.tsx

+  const [editModeValue, setEditModeValue] = useState(value || defaultValue);
+  const [readModeValue, setReadModeValue] = useState(value || defaultValue);


My 2c is I don't think we need to use value || here for our internal state. Since they're only used for uncontrolled behavior, they only need to listen for defaultValue.

Suggested change

const [editModeValue, setEditModeValue] = useState(value || defaultValue);

const [readModeValue, setReadModeValue] = useState(value || defaultValue);

const [editModeValue, setEditModeValue] = useState(defaultValue);

const [readModeValue, setReadModeValue] = useState(defaultValue);

This logic is actually how I'm able to pass readModeValue to the onCancel function. Otherwise, the readModeValue falls out of sync and isn't a reliable backup of the previous value.
Link to code sample mentioned: #7117 (comment)

Ahh super interesting! If readModeValue is the only one that needs this logic, let's set it just for that and add an inline comment noting why

cee-chen · 2023-08-23T23:33:56Z

src/components/inline_edit/inline_edit_form.tsx

+  const valueToUse = useMemo(() => {
+    if (value) {


[naming things is hard] how do you feel about renaming the consumer-passed value prop to value: controlledValue or possibly just _value, and then just naming this value?

cee-chen · 2023-08-23T23:34:33Z

src/components/inline_edit/inline_edit_form.tsx

+    if (value) {
+      return value;
+    } else {
+      return isEditing ? editModeValue : readModeValue || placeholder;


I still can't get over how elegant this ternary is haha. Super awesome!

cee-chen · 2023-08-23T23:34:38Z

src/components/inline_edit/inline_edit_form.tsx

            inputRef={setEditModeRefs}
            onChange={(e) => {
              setEditModeValue(e.target.value);
+              onChange && onChange(e);


quick nit while I'm here: onChange?.(e) for brevity

breehall · 2023-09-05T20:59:59Z

Closing this PR in favor #7157

breehall added 3 commits August 23, 2023 13:09

[EuiInlineEdit] Controlled component:

1dab8e3

- Add the onCancel callback prop to allow consumers to fallback to the previous inline edit value upon cancel - Create a DRY helper to switch between readModeValue, editModeValue, and value appropriately

[DEMO ONLY] Create a demo of controlled component usage inside of Eui…

51ad584

…Title

breehall commented Aug 23, 2023

View reviewed changes

cee-chen reviewed Aug 23, 2023

View reviewed changes

breehall closed this Sep 5, 2023

breehall mentioned this pull request Sep 5, 2023

[EuiInlineEdit] Allow EuiInlineEdit to be used as a controlled component #7157

Merged

8 tasks

breehall deleted the inline-edit/controlled branch October 6, 2023 16:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] [EuiInlineEdit] Allow `EuiInlineEdit` to be used as a controlled component#7117

[WIP] [EuiInlineEdit] Allow `EuiInlineEdit` to be used as a controlled component#7117
breehall wants to merge 3 commits intoelastic:mainfrom
breehall:inline-edit/controlled

breehall commented Aug 23, 2023 •

edited

Loading

Uh oh!

elasticmachine commented Aug 23, 2023 •

edited

Loading

Uh oh!

breehall Aug 23, 2023

Uh oh!

kibanamachine commented Aug 23, 2023

Uh oh!

cee-chen Aug 23, 2023

Uh oh!

cee-chen Aug 23, 2023

Uh oh!

cee-chen Aug 23, 2023 •

edited

Loading

Uh oh!

breehall Aug 25, 2023

Uh oh!

cee-chen Aug 25, 2023 •

edited

Loading

Uh oh!

cee-chen Aug 23, 2023

Uh oh!

cee-chen Aug 23, 2023

Uh oh!

cee-chen Aug 23, 2023

Uh oh!

breehall commented Sep 5, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		const [editModeValue, setEditModeValue] = useState(value \|\| defaultValue);
		const [readModeValue, setReadModeValue] = useState(value \|\| defaultValue);

Conversation

breehall commented Aug 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Feature Checklist

QA

General checklist

Uh oh!

elasticmachine commented Aug 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💔 Build Failed

Failed CI Steps

History

Uh oh!

breehall Aug 23, 2023

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Aug 23, 2023

Uh oh!

cee-chen Aug 23, 2023

Choose a reason for hiding this comment

Uh oh!

cee-chen Aug 23, 2023

Choose a reason for hiding this comment

Uh oh!

cee-chen Aug 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

breehall Aug 25, 2023

Choose a reason for hiding this comment

Uh oh!

cee-chen Aug 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cee-chen Aug 23, 2023

Choose a reason for hiding this comment

Uh oh!

cee-chen Aug 23, 2023

Choose a reason for hiding this comment

Uh oh!

cee-chen Aug 23, 2023

Choose a reason for hiding this comment

Uh oh!

breehall commented Sep 5, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

breehall commented Aug 23, 2023 •

edited

Loading

elasticmachine commented Aug 23, 2023 •

edited

Loading

cee-chen Aug 23, 2023 •

edited

Loading

cee-chen Aug 25, 2023 •

edited

Loading