2
\$\begingroup\$

I created a simple note-taking application, which enables a user to create notes, update them, and delete them. I used local storage for saving the notes. Would Appreciate some feedback on how to improve it.

This is the index.html

<!DOCTYPE html>
<html lang="en">

<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>Notes-App</title>
</head>

<body>
    <h1>Notes App</h1>
    <h2>Take notes</h2>
    <input id="search-text" type="text" placeholder="Filter Notes" />
    <select id="filter-by"> 
        <option value="byEdited">Sort By Last Edited</option>
        <option value="byCreated">Sort By Recently Created</option>
        <option value="alphabetical">Sort Alphabetically</option>
    </select>
    <div id="notes"></div>
    <button id="create-note">Create Note</button>
    <script src="https://unpkg.com/uuid@latest/dist/umd/uuidv4.min.js"></script>
    <script src="https://cdnjs.cloudflare.com/ajax/libs/moment.js/2.29.1/moment-with-locales.min.js"
        integrity="sha512-LGXaggshOkD/at6PFNcp2V2unf9LzFq6LE+sChH7ceMTDP0g2kn6Vxwgg7wkPP7AAtX+lmPqPdxB47A0Nz0cMQ=="
        crossorigin="anonymous"></script>
    <script src="notes-functions.js"></script>
    <script src="notes-app.js"></script>
</body>

</html>

This is the html for the edit.html page where you can edit the title and body

<!DOCTYPE html>
<html lang="en">

<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>Edit Page</title>
    <script src="https://unpkg.com/uuid@latest/dist/umd/uuidv4.min.js"></script>
</head>

<body>
    <a href="/index.html">Home</a>

    <input id="note-title" type="text" placeholder="Note Title" />
    <span id="last-edited"></span>
    <textarea id="note-body" placeholder="Enter text"></textarea>
    <button id="remove-note">Remove Note</button>
    <script src="https://cdnjs.cloudflare.com/ajax/libs/moment.js/2.29.1/moment-with-locales.min.js"
    integrity="sha512-LGXaggshOkD/at6PFNcp2V2unf9LzFq6LE+sChH7ceMTDP0g2kn6Vxwgg7wkPP7AAtX+lmPqPdxB47A0Nz0cMQ=="
    crossorigin="anonymous"></script>
    <script src="notes-functions.js"></script>
    <script src="notes-edit.js"></script>
</body>

</html>

This is the notes-app.js

    let notes = getSavedNotes();
    
    const filters = {
      searchText: "",
      sortBy: "byEdited",
    };
    
    renderNotes(notes, filters);
    
    document.querySelector("#create-note").addEventListener("click", (e) => {
      const id = uuidv4();
      const timestamp = moment().valueOf();
      notes.push({
        id,
        title: "",
        body: "",
        createdAt: timestamp,
        updatedAt: timestamp,
      });
      saveNotes(notes);
      //redirects user to edit page when new note is created
      location.assign(`edit.html#${id}`);
    });
    
    document.querySelector("#search-text").addEventListener("input", (e) => {
      filters.searchText = e.target.value;
      renderNotes(notes, filters);
    });
    
    document.querySelector("#filter-by").addEventListener("change", (e) => {
      filters.sortBy = e.target.value;
      renderNotes(notes, filters);
    });
    
    window.addEventListener("storage", (e) => {
      if (e.key === "notes") {
        notes = JSON.parse(e.newValue);
        renderNotes(notes, filters);
      }
    });

This is the file I created for functions for updating and modifying the notes, called `note-functions.js`

// Read exisiting notes from localstorage
const getSavedNotes = () => {
  const notesJSON = localStorage.getItem("notes");
  return notesJSON !== null ? JSON.parse(notesJSON) : [];
};

  const sortNotes = (notes, sortBy) => {
    if (sortBy === "byEdited") {
      return notes.sort((a, b) => {
        if (a.updatedAt > b.updatedAt) {
          return -1;
        } else if (a.updatedAt < b.updatedAt) {
          return 1;
        } else {
          return 0;
        }
      });
    } else if (sortBy === "byCreated") {
      return notes.sort((a, b) => {
        if (a.createdAt > b.createdAt) {
          return -1;
        } else if (a.createdAt < b.createdAt) {
          return 1;
        } else {
          return 0;
        }
      });
    } else if (sortBy === "alphabetical") {
      return notes.sort((a, b) => {
        if (a.title.toLowerCase() < b.title.toLowerCase()) {
          return -1;
        } else if (a.title.toLowerCase() > b.title.toLowerCase()) {
          return 1;
        } else {
          return 0;
        }
      });
    } else {
      return notes;
    }
  };


// Render application notes
const renderNotes = (notes, filters) => {
  notes = sortNotes(notes, filters.sortBy);
  const filteredNotes = notes.filter((note) => {
    return note.title.toLowerCase().includes(filters.searchText.toLowerCase());
  });

  document.querySelector("#notes").innerHTML = "";
  filteredNotes.forEach((note) => {
    const noteElement = generateNoteDom(note);
    document.querySelector("#notes").appendChild(noteElement);
  });
};

//Save Notes
const saveNotes = (notes) => {
  localStorage.setItem("notes", JSON.stringify(notes));
};

const removeNote = (id) => {
  const noteIndex = notes.findIndex((note) => {
    return note.id === id;
  });

  if (noteIndex > -1) {
    notes.splice(noteIndex, 1);
  }
};

// Generate the DOM structure for a note
const generateNoteDom = (note) => {
  const noteEl = document.createElement("div");
  const button = document.createElement("button");
  const textEl = document.createElement("a");

  //Setup delete button
  button.textContent = " x";
  noteEl.appendChild(button);
  button.addEventListener("click", () => {
    removeNote(note.id);
    saveNotes(notes);
    renderNotes(notes, filters);
  });

  //Setup note title
  if (note.title.length > 0) {
    textEl.textContent = note.title;
  } else {
    textEl.textContent = " Unamed note";
  }

  //When user clicks on note, it takes them to the edit page with
  //Corresponding ID
  textEl.setAttribute("href", `edit.html#${note.id}`);
  noteEl.appendChild(textEl);
  return noteEl;
};

const dataLastEdited = (timestamp) => {
  return `Last edited ${moment(timestamp).fromNow()}`;
};

and finally, this is the note-edit.js file which is responsible for the edit.html page

const noteID = location.hash.substring(1);
const noteTitle = document.querySelector("#note-title");
const noteBody = document.querySelector("#note-body");
const removeEl = document.querySelector("#remove-note");
const lastEdited = document.querySelector("#last-edited");
let notes = getSavedNotes();

let note = notes.find((note) => {
  return note.id === noteID;
});

//if note is not found then redirect user to index
if (note === undefined) {
  location.assign("/index.html");
}

noteTitle.value = note.title;
noteBody.value = note.body;
lastEdited.textContent = dataLastEdited(note.updatedAt);

noteTitle.addEventListener("input", (e) => {
  note.title = e.target.value;
  note.updatedAt = moment().valueOf();
  lastEdited.textContent = dataLastEdited(note.updatedAt);
  saveNotes(notes);
});

noteBody.addEventListener("input", (e) => {
  note.body = e.target.value;
  note.updatedAt = moment().valueOf();
  lastEdited.textContent = dataLastEdited(note.updatedAt);
  saveNotes(notes);
});

removeEl.addEventListener("click", () => {
  removeNote(note.id);
  saveNotes(notes);
  location.assign("/index.html");
});

window.addEventListener("storage", (e) => {
  if (e.key === "notes") {
    notes = JSON.parse(e.newValue);
    let note = notes.find((note) => {
      return note.id === noteID;
    });

    //if note is not found then redirect user to index
    if (note === undefined) {
      location.assign("/index.html");
    }
    noteTitle.value = note.title;
    noteBody.value = note.body;
  }
  lastEdited.textContent = dataLastEdited(note.updatedAt);
});
\$\endgroup\$
3
\$\begingroup\$

Script location The earlier external resources like <script src and <link tags come in the DOM, the earlier they'll be parsed by the browser, which means they can be downloaded more quickly by the client. If your scripts aren't written inline, then you can consider putting them in the <head> so that they're downloaded ASAP. In contrast, if you put a script at the bottom of the body, and the page is large enough that the whole HTML doesn't get downloaded immediately, the script will not even start to download until the final chunk of the HTML is parsed by the client.

(It very likely doesn't matter much for pages this small, but that could change if you decide to add more HTML content, and it's a good habit to get into regardless IMO)

To avoid having to wrap the script contents in a DOMContentLoaded listener, and to avoid render-blocking, you can add the defer attribute to all the script tags.

Concise sorting When you need to sort based on numeric differences, rather than writing out return 1; or return 0; or return -1 depending, you can simply return the difference (which will evaluate to a positive number, 0, or to a negative number, which ia all that's needed).

When you need to sort by whether a string comes before another alphabetically, use localeCompare:

const sortNotes = (notes, sortBy) => {
    if (sortBy === "byEdited") {
        return notes.sort((a, b) => b.updatedAt - a.updatedAt);
    } else if (sortBy === "byCreated") {
        return notes.sort((a, b) => b.createdAt - a.createdAt);
    } else if (sortBy === "alphabetical") {
        return notes.sort((a, b) => a.toLowerCase().localeCompare(b.toLowerCase()));
    } else {
        return notes;
    }
};

Styles? Adding CSS styles to webpages will help it look a lot less plain.

Sorting sorts in-place - when you do

const renderNotes = (notes, filters) => {
    notes = sortNotes(notes, filters.sortBy);

you reassign notes to the same array it refers to originally. Consider leaving out the reassignment:

const renderNotes = (notes, filters) => {
    sortNotes(notes, filters.sortBy);

Consider implicit return I notice that despite using arrow functions, you're never using implicit return. If this is a deliberate style choice, that's fine. If not, consider where it might be worth it - for example, if you so wished, this:

const noteIndex = notes.findIndex((note) => {
    return note.id === id;
});

could be

const noteIndex = notes.findIndex(note => note.id === id);

(If note lookup by ID is a frequent operation, you could consider an object indexed by ID instead of an array in order to look up items faster, though it'd make sorting a bit less natural since you'd have to extract the object values into an array first)

Variable name There's const textEl = document.createElement("a");, but text nodes are a separate concept. Maybe call it editLink or editAnchor instead. Also, the function dataLastEdited returns a last edited timestamp string for display, so perhaps call it dateLastEdited.

Terminate the script on the edit page if a note isn't found This:

//if note is not found then redirect user to index
if (note === undefined) {
    location.assign("/index.html");
}

noteTitle.value = note.title;
// ...

can be

//if note is not found then redirect user to index
if (note === undefined) {
    location.assign("/index.html");
} else {
    noteTitle.value = note.title;
    // ...

or, in a function:

if (note === undefined) {
    location.assign("/index.html");
    return;
}
noteTitle.value = note.title;
// ...

You're properly setting values from the user into textContent rather than concatenating into an HTML string or using .innerHTML, which will successfully prevent XSS-related exploits. This is a very common issue that people get wrong, it's nice to see it implemented properly.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.