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

1 stars 3 forks source link

Hulya and Isa sorting items #38

Closed isaabutaa closed 3 years ago

isaabutaa commented 3 years ago

Description

Related Issue

closes #12

Acceptance Criteria

Type of Changes

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

Updates

Before

sorted-collection

After

Hover over elements to see accessibility attributes: Screenshot (8)

Sorted Items with color: sorted-items

Enable voiceover on Mac:

Screen Shot 2021-08-17 at 5 44 19 PM

Testing Steps / QA Criteria

github-actions[bot] commented 3 years ago

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

https://tcl-28-shopping-list--pr38-hk-ia-sort-shopping-e3l98g3z.web.app

(expires Fri, 27 Aug 2021 00:46:59 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

hulyak commented 3 years ago

Great job with this @hulyak @isaabutaa! I pulled down the branch and everything worked as expected when I added new items to my list.

I encountered a bug when I first opened my list, some items for my token weren't displaying at all, but they were all old items added a while ago that didn't contain all of the fields we now include (I think the one causing the issue is daysUntilPurchase, which wasn't included in older items). I deleted them from Firestore and added new items instead and all worked as expected. If it's ok with everyone, I'm happy to go back and filter through old items in Firestore on my own time and remove them so we don't run into this issue going forward.

Yes, I think we should clean up Firestore 😃 I tried to delete some of them, but couldn't check all.

scooberu commented 3 years ago

My personal recommendation for getting the sorting correct would be to create a Map/Dict object to store the firestore query results, with that Map containing an array for each of our groups (soon, not soon, kind of soon, and inactive) (itself sorted alphabetically). But my JS practicum is wanting; not sure the best way to handle this idiomatically. @luisaugusto / @meganesu I would be interested to hear your feedback on the best JSish way to handle the interior sorting among these groups.

scooberu commented 3 years ago

Just noting here that we realized the AC had already been met with the way the sorting worked already; disregard my last.