Jo qg sort shopping list #30
Conversation
|
Visit the preview URL for this PR (updated for commit f749273): https://tcl-79-smart-shopping-list--pr30-jo-qg-sort-shopping-vj0pf99x.web.app (expires Thu, 26 Sep 2024 09:58:27 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: d91d9ddbda780208241c52942f544acf8e81407a |
There was a problem hiding this comment.
Great job with this tough issue impressive work! 👏. I left suggestions as comments i believe addressing them would enhance performance and improve clarity.
I also noticed that the indicators disappear when filtering with the search bar and they don’t reappear after clearing the filter since data goes back to before the indicators are added.
src/api/firebase.js
Outdated
| export function comparePurchaseUrgency(arr) { | ||
| const soonArray = arr | ||
| .filter((item) => item.indicator === 'Soon') | ||
| .sort((a, b) => (a.name > b.name ? 1 : -1)); | ||
| const kindOfSoon = arr | ||
| .filter((item) => item.indicator === 'Kind of soon') | ||
| .sort((a, b) => (a.name > b.name ? 1 : -1)); | ||
| const notSoon = arr | ||
| .filter((item) => item.indicator === 'Not soon') | ||
| .sort((a, b) => (a.name > b.name ? 1 : -1)); | ||
| const inactive = arr | ||
| .filter((item) => item.indicator === 'Inactive') | ||
| .sort((a, b) => (a.name > b.name ? 1 : -1)); | ||
| const overdue = arr | ||
| .filter((item) => item.indicator === 'Overdue') | ||
| .sort((a, b) => (a.name > b.name ? 1 : -1)); | ||
| return overdue.length > 0 | ||
| ? overdue.concat(soonArray, kindOfSoon, notSoon, inactive) | ||
| : soonArray.concat(kindOfSoon, notSoon, inactive); | ||
| } |
There was a problem hiding this comment.
I believe grouping the items by their indicators in a single loop by using an object to hold the arrays for each indicator, then sort and concatenate them after that could really enhance both performance and readability.
src/views/List.jsx
Outdated
|
|
||
| export function List({ data, listPath }) { | ||
| const [search, setSearch] = useState(''); | ||
| const [displayData, setDisplayData] = useState([]); | ||
| const navigate = useNavigate(); | ||
|
|
||
| useEffect(() => { | ||
| setDisplayData([...data]); | ||
| const getIndicator = (item) => { |
There was a problem hiding this comment.
moving getIndicator to utils would improve the organization.
src/views/List.jsx
Outdated
| const dateLastPurchasedJavaScriptObject = item.dateLastPurchased | ||
| ? item.dateLastPurchased.toDate() | ||
| : item.dateCreated.toDate(); | ||
|
|
||
| const daysSinceLastPurchase = getDaysBetweenDates( | ||
| new Date(), | ||
| dateLastPurchasedJavaScriptObject, | ||
| ); |
There was a problem hiding this comment.
Since we used the daysSinceLastPurchase logic twice now, it could be useful to create a function for it in the utils to use in both getIndicator and updateItem.
| } else if (daysSinceLastPurchase > 30 && daysSinceLastPurchase < 60) { | ||
| return 'Overdue'; |
There was a problem hiding this comment.
i believe when dateNextPurchasedhas passed is how an item becomes Overdue.
| setAllData([...comparePurchaseUrgency(arrayWithIndicator)]); | ||
| setDisplayData([...comparePurchaseUrgency(arrayWithIndicator)]); |
There was a problem hiding this comment.
i think its best to use a variable to be stored in the states instead of calling comparePurchaseUrgency twice.
Description
Code implements a sort function to sort our shopping list items by urgency and alphabetical order. Overdue items are pushed to the top of the list and inactive items are pushed to the bottom.
Related Issue
Closes #13
Acceptance Criteria
api/firestore.jsexports a newcomparePurchaseUrgencyfunction with the following behaviorsType of Changes
FeatureUpdates
Before
After
Testing Steps / QA Criteria
git pullandgit checkout jo-qg-sort-shopping-listnpm startlistpage