-
Notifications
You must be signed in to change notification settings - Fork 2
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
13. As a user, I want to view a list of my shopping list items in order of how soon I am likely to need to buy each of them again so that it’s clear what I need to buy soon #37
Conversation
Visit the preview URL for this PR (updated for commit dde5e5a): https://tcl-66-smart-shopping-list--pr37-ec-hm-issue-13-b4dnxh6w.web.app (expires Thu, 21 Mar 2024 16:32:18 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 2dc16cb2a79cbd6723fdc511b73e0743df1d18d0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work team Devs! Nice algorithm! The updated variable names are well thought out as well. 💣💣
🙋🏽♀️ I had one comment added in the firebase.js file.
Also had a thought--- I have a lot to learn about this, but thinking about a sorting algorithm made me wonder if in the future we should consider an item limit on our lists. I think it would take more than a person would logically use to overwhelm this algorithm, but it might be a good look ahead to consider.
Nice job y'all. Code looks clean. Added some comments about some naming stuff, but not imperative that we change things. My only other thing, which is not part of the AC, but could be something to think about, is that maybe in the future we could add a key that has information on what each colour/timeframe mean. We could also have maybe a hover/click effect where the word descriptor would appear, but otherwise they could just be by colour. Obviously this is not part of your scope, but I am just thinking for a nice minimal look it could be cool. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything seems to be working as expected. Nice work, looks good to me!
Description
todaysDate
variable into dates.js from ListItem.jsx and then exported it from dates.js because it was used in different files.subtractDates
function in dates.js to begetDifferenceBetweenDates
because it needed to be used in multiple places.subtractDates
into theisChecked
function in ListItem.jsx to be the new value ofchecked
on the input.comparePurchaseUrgency
, to compare the purchase urgency of each shopping item. It sorts the item based on the acceptance criteria below.Related Issue
Closes #13
Acceptance Criteria
api/firestore.js
exports a newcomparePurchaseUrgency
function with the following behaviorsA stretch goal
If you complete all of the previous acceptance criteria, consider what happens when an item’s
dateNextPurchased
has passed, but it isn’t yet inactive. Let’s call that item “overdue”.comparePurchaseUrgency
to sort “overdue” items to the top of the listType of Changes
Updates
Before
After
Testing Steps / QA Criteria
dateNextPurchased
to something less than today's date (less than 0 days between dates) anddateLastPurchased
needs to be greater thandateNextPurchased
dateLastPurchased
to a date that is older than 60 daysname
prop in thelabel
element in ListItem.jsx.