-
Notifications
You must be signed in to change notification settings - Fork 4
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
Program tag components #133
Conversation
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.
Looks really good! I think the filter using Chip components also need to be refactored still - probably add a boolean for tag
as a prop and then switch the styles/color attributes of the Chip
on that?
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.
Looks good! Just make sure to address Annie's comment about refactoring the filter chips and I think it's good to go.
// i.e. "WIC" was entered as "WIc" or "SNAP Match" was entered as "snap Match" | ||
// Hopefully ensures that chips will work most of the time, only not displaying if | ||
// they enter the wrong string of words | ||
if ( |
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.
Nice that you considered these cases, but wouldn't this input only come from within our code? Why would we need this here?
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.
I figured that the program name gets passed from so many sources (i.e. store card, store details, store list and maybe more in the future) so if there's ever a case someone spells it wrong or something it would still work ?? it's a bit extra but i guess it ensures that the chips still function correctly even if there's a small capitalization error in the code
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.
i agree with this sentiment much human error
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.
Looks great! So much cleaner 💯
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.
Looks good!!
What's new in this PR
How to review
Verify that chips still appear correctly in the
StoreCard
andStoreDetailsScreen
andStoresList
.Verify that stores like "BP Blair" have the default text for no programs available.
Relevant Links
Online sources
Related PRs
#98
Next steps
Screenshots
CC: @anniero98 @wangannie