ZachYanez / SayHey

Here is a progressive web app I built to make it easier to communicate with people who might be 6ft away. Type in a message and see it displayed across the screen in a large font. Save presets to quickly access common messages
https://sayheyapp.link
1 stars 0 forks source link

Getting key for delete #6

Open ZachYanez opened 3 years ago

ZachYanez commented 3 years ago

@danielrobertson So the delete() function expects the id as the parameter, but just having trouble grabbing it from the parent element.

Working on Modalbranch in src/components/main

danielrobertson commented 3 years ago

I totally see what you were attempting, by just passing the DeletePresent function to the onClick and letting the browser API pass the target.event value , but this doesn't work since the browser will pass the event.target for the span, not the parent button which actually has the event.target you want

Easy fix since we have the message in scope in the map loop, so let's forgo browser event delegation and get manual with passing the message instead

<span
    onClick={() => {
        DeletePreset(message);
    }}
    className="deleteBtn"
    as="button"
>
    x
</span>

And our deletePreset function can simply accept the message, look up the DB id for the record we're trying to match message on, and pass that ID to deleteRecord

  function DeletePreset(message) {
    getAll().then((dbRecords) => {
      for (const record of dbRecords) {
        if (record.message === message) {
          deleteRecord(record.id).then((event) => {
            console.log("Deleted Preset");
            alert("Deleted!");
          });
          console.log("Deleted Preset");
        }
      }
    });
  }

That works! 🎉 You could call it a day here for this project since presets will probably only ever be a handful at most.

But it's not scalable beyond maybe a few hundred records in theory, since you'll be looping over the whole DB every time. But we have to do this since presets[] is just a list of the messages, not their DB ids. A solution to this inefficiency is to rearchitect the setup a bit such that presets[] is a list of objects, which resembles the DB record structure, presets = [{id: .., message: ...}] and in this way, our render code would become

{presets.map((preset) => {
    const {id, message} = preset;
    return (
    <Dropdown.Item
    key={message}
    onClick={handleChange}
    value={message}
    type="text"
    as="button"
    >
    {message}
    <span
        onClick={() => {
          DeletePreset(id);
        }}
        className="deleteBtn"
        as="button"
    >
        x
    </span>
    </Dropdown.Item>
)})}

and then deletePreset would take the ID and call deleteRecord right away without having to loop over the DB.

Random thoughts

ZachYanez commented 3 years ago

@danielrobertson Amazing response. Thank you! I liked the scalable solution. I tried to refactor it a little bit, but I'm not sure where I would go to accomplish what you said at "rearchitect the setup" of presets[]. What I have now successfully renders the messages

Screen Shot 2021-02-12 at 11 54 43 AM

danielrobertson commented 3 years ago

Appending this to my original response, cause I just realized that while it correctly deletes presets from the DB, it doesn't update the presets state. You can do this trick of copying the array and splicing it to accomplish this, and now the UI presets dropdown stays up-to-date with indexDB as the user deletes presets.

 function DeletePreset(message) {
    getAll().then((dbRecords) => {
      for (const record of dbRecords) {
        if (record.message === message) {
          deleteRecord(record.id).then((event) => {
            alert("Deleted!");
          });
        }
      }
    });

    setPresets((prevPresets) => {
      const presetsCopy = [...prevPresets];
      const numberToDelete = 1; 
      const indexOfMessageToDelete = presetsCopy.indexOf(message); 
      presetsCopy.splice(indexOfMessageToDelete, numberToDelete);
      return presetsCopy;
    });
  }
danielrobertson commented 3 years ago

@danielrobertson Amazing response. Thank you! I liked the scalable solution. I tried to refactor it a little bit, but I'm not sure where I would go to accomplish what you said at "rearchitect the setup" of presets[]. What I have now successfully renders the messages

It's going to touch almost every bit of this component to rearchitect it. Here's what I had in mind, and I'm pasting almost the entire component here since it is many little, intertwined changes, but the notable changes are

Main.js

export default function Main() {
  const INITIAL_PRESETS = [{ id: -1, message: "Hey" }];
  const { add, getAll, deleteRecord } = useIndexedDB("presets");

  const [message, setMessage] = useState();
  const [preset, setPreset] = useState("");
  const [presets, setPresets] = useState(INITIAL_PRESETS);

  useEffect(() => {
    getAll().then((presetDbDocument) => {
      if (presetDbDocument) {
        console.log(
          "presets found in DB, adding to list: ",
          presetDbDocument.message
        );
        const indexedPresets = presetDbDocument;
        setPresets([...INITIAL_PRESETS, ...indexedPresets]);
      }
    });
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, []);

  function handleChange(e) {
    e.preventDefault();
    setMessage(e.target.value);
    console.log(e.target.value);
  }

  function handlePreset(e) {
    setPreset(e.target.value);
  }

  const [show, setShow] = useState(false);

  const handleClose = () => setShow(false);
  const handleShow = () => setShow(true);

  function AddPresets() {
    add({ message: preset }).then(
      (generatedId) => {
        console.log("Preset Indexed: ", { id: generatedId, message: preset });

        setPresets((prevPresets) => {
          return [...prevPresets, { id: generatedId, message: preset }];
        });
      },
      (error) => {
        console.log(error);
      }
    );

    handleClose();
  }

  function deletePreset(id) {
    deleteRecord(id).then(() => {
      alert(`Deleted ${id}`);
    });

    setPresets((prevPresets) => {
      const presetsCopy = [...prevPresets];
      const indexOfRecordToDelete = presetsCopy.findIndex((p) => p.id === id);
      const howManyToDelete = 1;
      presetsCopy.splice(indexOfRecordToDelete, howManyToDelete);
      return presetsCopy;
    });
  }

  return (
    <div>
      <Navbar bg="l" variant="light" fixed="top">
        <Navbar.Brand>sayHey</Navbar.Brand>
        <Nav className="mr-auto">
          <DropdownButton id="dropdown-item-button" title="Presets">
            <Dropdown.Item onClick={handleShow} type="text" as="button">
              Add Preset +
            </Dropdown.Item>
            <Modal show={show} onHide={handleClose}>
              <Modal.Header closeButton>
                <Modal.Title>Add Preset</Modal.Title>
              </Modal.Header>
              <Modal.Body>
                <InputGroup className="mb-3">
                  <FormControl
                    onChange={handlePreset}
                    value={this}
                    placeholder="Message"
                    aria-label="Message Preset"
                    aria-describedby="basic-addon2"
                  />
                  <InputGroup.Append>
                    <InputGroup.Text onClick={AddPresets} id="basic-addon2">
                      Save
                    </InputGroup.Text>
                  </InputGroup.Append>
                </InputGroup>
              </Modal.Body>
            </Modal>
            {presets.map((preset) => {
              const { id, message } = preset;
              return (
                <Dropdown.Item
                  key={message}
                  onClick={handleChange}
                  value={message}
                  type="text"
                  as="button"
                >
                  {message}
                  <span
                    onClick={() => {
                      deletePreset(id);
                    }}
                    className="deleteBtn"
                    as="button"
                  >
                    x
                  </span>
                </Dropdown.Item>
              );
            })}
          </DropdownButton>
........
danielrobertson commented 3 years ago

Totally optional and a matter of preference, but the Promises stuff in the useEffect and addPresets can be cleaned up using async await patterns

  useEffect(() => {
    const populateStateWithDbRecords = async () => {
      const records = await getAll();
      console.log("records found in DB, udpating state: ", records);
      setPresets([...INITIAL_PRESETS, ...records]);
    };

    populateStateWithDbRecords();
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, []);
  async function addPreset() {
    try {
      const generatedId = await add({ message: preset });
      console.log("Added to db: ", { id: generatedId, message: preset });

      setPresets((prevPresets) => {
        return [...prevPresets, { id: generatedId, message: preset }];
      });
    } catch (err) {
      console.error(err);
    }

    handleClose();
  }
danielrobertson commented 3 years ago

RE: the oudated documentation for react-indexed-db

Looks like it's somewhat maintained so you're good to keep using it but I went ahead and submitted a pull request to update the docs if the maintainers want to. Yay #opensource

This is definitely a problem with picking a library on npm, you kind of have to be critical of which library to use if multiple offer similar functionalities, by checking how many users, maintainers, how frequently it's updated, etc

ZachYanez commented 3 years ago

I love that you found their oversight😂 I had to run an errand but will look at what you just wrote when I get back home

ZachYanez commented 3 years ago

@danielrobertson Man, I can't tell you how much this means to me to have a friend help me out with coding. With the fall of the music industry around here this is truly a lifeline for me. Thank you for your help. It means a lot.

Also, I love to study your code. I feel like I'm learning just from reading it over and over.