Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Feat/add item quantity #46
Feat/add item quantity #46
Changes from all commits
c27cfb9
e6a00cf
f2789d6
f175b7b
170d3e3
912f056
2097649
7c848c3
153c3f0
266c2cc
ed049b5
c41b19a
dd7d557
f4eb386
ccde792
cb1804c
49c2029
9d5342b
ff80a47
0dd580d
1755648
94bd6df
0781346
9a8cd06
1c48aaa
006fc39
a20b47d
a46682a
d8ca614
6d98f71
f5f9ee6
eb19b32
9e7d038
d42c05a
5beeed5
1883d22
b3ac8ef
1ee02d3
0778dd5
525ac74
b582e82
5427158
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
suggestion: i think we could move this logic for updating the item quantity into the
ItemQuantityForm
. reading through the code right now it feels like there is some duplication of logic, kind of adding logic to components to make another component work.moving this to be handled by the
ItemQuantityForm
it could make it a bit more reusable because we could just place it into the location it is needed and not write logic within that component to get it to work. it would be like this check box and other forms the logic to do the updating and steps the form is doing is housed within the logic.To get the
item
prop needed with these changes on the list view we could try moving the<ItemQuantityForm saveItemQuantity={editItemQuantity} item={item} /> x{" "}
call into theList
component with this logicand I believe it would get the same result.
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 idea, that way the logic outside of firebase for quantity is all referable in one place.
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.
More a comment for later if we ever do a refactor. It may still be beneficial to move the logic for updating the item quantity to be contained in the
ItemQuanitiyForm
Nonblocking!
Awesome functionality to add to the app!