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

1 stars 3 forks source link

Sandy and Isa - Add new item #21

Closed isaabutaa closed 3 years ago

isaabutaa commented 3 years ago

For an example of how to fill this template out, see this Pull Request.

Description

Related Issue

4

Acceptance Criteria

Type of Changes

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

Updates

Before

image

Screen Shot 2021-07-18 at 10 32 54 PM

After

add-item-form

item-list

Testing Steps / QA Criteria

github-actions[bot] commented 3 years ago

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

https://tcl-28-shopping-list--pr21-sd-ia-addnewitem-mj42cymn.web.app

(expires Thu, 29 Jul 2021 00:29:15 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

meganesu commented 3 years ago

Should we refactor it so that there is a default selection (and frequency) that is 7, 14, or 30 days?

Following up on this, plus @isaabutaa's suggestion on Slack: "If instead of giving it a default value of 0 or 7, maybe we can have a null default value if it's just a one time purchase. And then if they select another value, it'll swap out the default value."

To start with the simplest solution, I'd suggest making the field required (either by picking one of the existing values as a default for now, or preselecting one of the radio buttons when the form loads). And then you can create an additional issue to improve that behavior later on, if we have time. (That issue will also be a good place to track conversation about possible approaches!)

Great collaboration, team! 💪 🥳

sandaiiyahh commented 3 years ago

As I was working on adding the id's, I realized we should also add an error handler for when a user submits an empty item input, which we can definitely implement in the future!

isaabutaa commented 3 years ago

As I was working on adding the id's, I realized we should also add an error handler for when a user submits an empty item input, which we can definitely implement in the future!

Good idea, Sandy!