Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delete button #35

Open
oliverjam opened this issue Apr 3, 2020 · 0 comments
Open

Delete button #35

oliverjam opened this issue Apr 3, 2020 · 0 comments
Assignees

Comments

@oliverjam
Copy link

oliverjam commented Apr 3, 2020

You have a couple of similar issues to some teams last week.

Button click-handlers

You should always attach click-handlers to buttons. They are the main semantic interactive element in HTML, they are focusable and have keyboard-support built-in. Adding a click-handler to a parent element and checking the tagName is not necessary, and probably a worse experience for some users.

You also don't ever send a response, which means the fetch request in the browser will never end. Since you refresh the page straight away this kind of doesn't matter, but it means you have an active request hanging around on your server until it times out. You should always set a status code and call response.end(), even if you're returning no body.

fetch callback

You aren't actually passing a function to the fetch's .then. The parentheses after the window.location.reload() mean that will execute immediately (i.e. as soon as the user clicks the button). This means your delete request is racing against your page reload, so it's possible the homepage willl load before the delete completes and you'll see an incorrect page.

You want to pass a function to: .then(() => window.location.reload()).

Error-handling

Your deleteHandler could do with some error-handling to avoid unhandled promise rejections if something goes wrong. Ideally you need both a .catch on the DB query and a request.on("error").

My suggestion

There's a simpler way that doesn't require any client-side JS at all: send a GET /delete-post?id=1 request using a standard anchor tag. You can server render the anchor tags with the POST ID inside your makeArticle template, using obj.id. So instead of a button you'd have an <a href="/delete-post?id=${obj.id}">. Then you can update your router to use that URL for the deleteHandler

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants