search

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

https://tcl-67-smart-shopping-list.web.app/
3 stars 1 forks source link

Issue 13 refinement #62

Closed flbarfield closed 5 months ago

flbarfield commented 6 months ago

Description

There were a few bits of remaining feedback that didn't get implemented before the end of the week.

Related Issue

closes issue 13 refinement

Type of Changes

Enhancement

Testing Steps / QA Criteria

This was mostly refactoring. But inactive and overdue items should now be sorted alphabetically.

github-actions[bot] commented 6 months ago

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

https://tcl-67-smart-shopping-list--pr62-issue-13-refinement-mabk8web.web.app

(expires Thu, 28 Mar 2024 00:24:02 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 1dc6f6876568bd8a1c97781eec7984835c207f7c

aberger3647 commented 6 months ago

Are active items sorting alphabetically? Screenshot 2024-03-20 at 12 35 16 PM See this screenshot...all items are "purchase soon" but fork is in the middle of the P's

flbarfield commented 5 months ago

Are active items sorting alphabetically? Screenshot 2024-03-20 at 12 35 16 PM See this screenshot...all items are "purchase soon" but fork is in the middle of the P's

Ah yes, I wanted to talk about this! I believe in the original issue, items were only supposed to sort alphabetically if they had the same number of daysBetween. But I think that's confusing to the user without actually displaying the daysBetween as a visual metric

So I'd like to propose just a straight alphabet sort for inactive/overdue/and active without that extra condition.

aberger3647 commented 5 months ago

If an item is categorized “purchase soon” doesn’t it have the same number of days between?

On Wed, Mar 20, 2024 at 12:49 PM Fredrick Barfield @.***> wrote:

Are active items sorting alphabetically? [image: Screenshot 2024-03-20 at 12 35 16 PM] https://private-user-images.githubusercontent.com/112574259/314606761-e94a5751-46a9-4269-b6a4-459f5fa283d0.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTA5NTY5MjAsIm5iZiI6MTcxMDk1NjYyMCwicGF0aCI6Ii8xMTI1NzQyNTkvMzE0NjA2NzYxLWU5NGE1NzUxLTQ2YTktNDI2OS1iNmE0LTQ1OWY1ZmEyODNkMC5wbmc_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjQwMzIwJTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI0MDMyMFQxNzQzNDBaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT00MjI0ZGFlYmU1NTBhMDhkODQzZTcyNDk0OWM5YTU4ZjhlZTQwMmZmNmIwNTUzNDVkMGNiNmJlODUxODViZjllJlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCZhY3Rvcl9pZD0wJmtleV9pZD0wJnJlcG9faWQ9MCJ9.Ylt8fbyu4CqYMRxPGldAMCTRSXyiKNk8otIXq7i2-7k See this screenshot...all items are "purchase soon" but fork is in the middle of the P's

Ah yes, I wanted to talk about this! I believe in the original issue, items were only supposed to sort alphabetically if they had the same number of daysBetween. But I think that's confusing to the user without actually displaying the daysBetween as a visual metric

So I'd like to propose just a straight alphabet sort for inactive/overdue/and active without that extra condition.

— Reply to this email directly, view it on GitHub https://github.com/the-collab-lab/tcl-67-smart-shopping-list/pull/62#issuecomment-2010240177, or unsubscribe https://github.com/notifications/unsubscribe-auth/A2236MYKDRVOENZ2YW3XGC3YZHDY3AVCNFSM6AAAAABE6WGT2KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMJQGI2DAMJXG4 . You are receiving this because your review was requested.Message ID: @.***>

flbarfield commented 5 months ago

Not necessarily. DaysBetween returns an integer in the date utils I believe. And then we have a seperate function with a condition that says it that integer is within a certain range, then its inactive/active/overdue.

aberger3647 commented 5 months ago

I see. I interpreted it as items in the same urgency should be alphabetized , but that’s not what is specified. It looks odd in the UI

On Wed, Mar 20, 2024 at 1:00 PM Fredrick Barfield @.***> wrote:

Not necessarily. DaysBetween returns an integer in the date utils I believe. And then we have a seperate function with a condition that says it that integer is within a certain range, then its inactive/active/overdue.

— Reply to this email directly, view it on GitHub https://github.com/the-collab-lab/tcl-67-smart-shopping-list/pull/62#issuecomment-2010271455, or unsubscribe https://github.com/notifications/unsubscribe-auth/A2236M4CMZEM4CX4JVLRQNTYZHFDHAVCNFSM6AAAAABE6WGT2KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMJQGI3TCNBVGU . You are receiving this because your review was requested.Message ID: @.***>

flbarfield commented 5 months ago

Agreed! I'll swap it around and push it when I have a chance then. Way easier to implement that way anyway, the other way was a headache 🤣