the-collab-lab / tcl-28-smart-shopping-list

1 stars 3 forks source link

Ia ks checkbox purchase #29

Closed ksiman14 closed 3 years ago

ksiman14 commented 3 years ago

Description

Related Issue

closes #8

Acceptance Criteria

Type of Changes

Type
:bug: Bug fix
:sparkles: New feature
:hammer: Refactoring
:100: Add tests
:link: Update dependencies
:scroll: Docs

Updates

Before

before-image

After

items-checkboxes

items-checked

Testing Steps / QA Criteria

To test 24 hour clearing:

github-actions[bot] commented 3 years ago

Visit the preview URL for this PR (updated for commit a2c7a42):

https://tcl-28-shopping-list--pr29-ia-ks-checkbox-purch-sva05n88.web.app

(expires Thu, 12 Aug 2021 19:19:21 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

ksiman14 commented 3 years ago

Wow, I personally find calculating dates and times intimidating too, so great job to both of you for tackling this! Everything works on my end, and isPurchased was changed to false and the box unchecked itself after I manually changed the lastPurchasedDate. Love the use for the new component too.

As a side note, I did get a linter warning for the useEffect on the SingleItem component, because we used async in its callback so I think it might return a promise? Usually, I would create a separate async function and invoke it, but I don't know how much of a difference it makes!

Good point on the async within useEffect, I am going to modify and push up changes now.

ksiman14 commented 3 years ago

Hey nice job! The only comment I have is that the checkbox will only update if you either navigate away from or refresh the page, so staying on the page won't trigger a refresh, which is probably an edge case. If you did want to do that, you can use something like setInterval to check the expiration date every 30 seconds so that users would get immediate feedback about their items.

This is a great idea! I'm including this now in a new commit.