giorgosart/react-easy-edit

Using editMode with default value

Closed this issue ยท 13 comments

Describe the bug
When using editMode and value has some text. then you can't change text no more.
It works fine if the value is empty.

To Reproduce
Steps to reproduce the behavior:
look to this code it reproduce the issue.

function App() {
const [edit, setEdit] = useState(false);
return (
<><EasyEdit
type={Types.TEXT}
value="I have custom buttons!"
onSave={(val) => console.log(val)}
editMode={edit}
onCancel={() => setEdit(false)}
/>
<button onClick={() => setEdit(!edit)}>Edit Mode
</>
);

Expected behavior
you can change text normally.

I'm also experiencing this.

Updates? This may end up being a showstopper for us, with little time to dig into the source.

@spacecat @jonmadison-amzn @cepres-nagi-ramadan
I think the problem here is that you have the "Save" button enabled which belongs to a child component of EasyEdit. When you click Save, you are trying to change the state of the parent component which is not working. Do you really need the save button? See below how i use this feature.

<h3>Toggle edit mode</h3>
                <button onClick={() => {this.setState({editMode: !this.state.editMode})}}>Toggle</button>
                <EasyEdit
                    type={Types.TEXT}
                    onSave={() => {
                      console.log("saved!")
                    }}
                    editMode={this.state.editMode}
                    hideCancelButton
                    hideSaveButton
                />
                <EasyEdit
                    type={Types.TEXT}
                    onSave={() => {
                      console.log("saved!")
                    }}
                    editMode={this.state.editMode}
                    instructions={"Toggle me!"}
                    hideCancelButton
                    hideSaveButton
                />
              </div>

I will take a look there is clearly a big with this.

i am looking into this, will be back with feedback

I prepared propose of solution -> #180

@spacecat @jonmadison-amzn @cepres-nagi-ramadan this is partially fixed in 1.17.1.

Thank you @pavelee for your contribution.

@giorgosart I think we could close the issue.

@pavelee the reason i haven't closed this bug yet is because of the behaviour below. I didn't get a chance to look into it but the "Save" button becomes unclickable under certain scenarios. Ideally the Save button should not be there as it doesn't make sense to have an external button controlling the edit state of one or more EasyEdit components but it's still a bug. Let me know if you will get any time to look into this if you can.

Media1.mp4

I will look into it ๐Ÿ‘, thanks for the great bug ๐Ÿ› example! ๐Ÿ™

@giorgosart I debugged the problem.

Here we have here problem with desynchronized internal and external state. EasyEdit cancel editMode onSave event, so we should keep the same logic with external state.

So to fix the issue we should update client code:

import React, { useState } from "react";
import "./App.css";
import EasyEdit, { Types } from "../lib/EasyEdit";

function App() {
  const [edit, setEdit] = useState(false);
  return (
    <>
      <EasyEdit
        type={Types.TEXT}
        value="text 1"
        onSave={(val) => {
          console.log(val);
          setEdit(false); // here we keep logic from EasyEdit, leave edit mode on save
        }}
        editMode={edit}
        onCancel={() => setEdit(false)}
      />
      <button onClick={() => setEdit(!edit)}>Edit Mode</button>
    </>
  );
}

export default App;

But here we could see that we got much bigger problem, because do we solve the issue we have here? As I understand we want to keep editMode as external state and toggle it. Not letting buttons to turn off editMode by them self.

Here example to better show my perspective:

import React, { useState } from "react";
import "./App.css";
import EasyEdit, { Types } from "../lib/EasyEdit";

function App() {
  const [edit, setEdit] = useState(false);
  return (
    <>
      <EasyEdit
        type={Types.TEXT}
        value="text 1"
        onSave={(val) => {
          console.log(val);
          setEdit(false);
        }}
        editMode={edit}
        onCancel={() => setEdit(false)}
      />
      <EasyEdit
        type={Types.TEXT}
        value="text 2"
        onSave={(val) => {
          console.log(val);
          setEdit(false);
        }}
        editMode={edit}
        onCancel={() => setEdit(false)}
      />
      <EasyEdit
        type={Types.TEXT}
        value="text 3"
        onSave={(val) => {
          console.log(val);
          setEdit(false);
        }}
        editMode={edit}
        onCancel={() => setEdit(false)}
      />
      <button onClick={() => setEdit(!edit)}>Edit Mode</button>
    </>
  );
}

export default App;

Do we here expect that clicking save on any button will leave editMode for every other input?

Maybe we should go in other direction, not letting save button to toggle editMode? So we can keep them in edit mode as long as parent component want.

We could solve this cleaner by adding setEditMode to props, so EasyEdit can synchronize parent state by them self.

But still we should consider how EasyEdit should work with external editMode state.

@pavelee I agree that the client code should handle this, happy to close the issue now. In my opinion, when you use an external button to control the state the of one or more EasyEdits, the Save button should be hidden using the prop responsible for that.