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

1 stars 2 forks source link

Filter To Locate Item On Existing List #25

Closed jssckbl closed 3 years ago

jssckbl commented 3 years ago

Description

This week, the main point of the story is the following: As a user, I want to filter my shopping list to make it easier to locate an item in the list.

All code changes are contained within the /List.js component. We were able to build off of last week's .map by utilizing the filter() method. We also utilized setState. Since we are simply needing to make a request, rather than needing to apply any changes to the Firestore database during this list filtration, utilizing setState seemed like a good option to use.

In addition, we addressed accessibility for the additions for this AC.

Related Issue

closes https://github.com/the-collab-lab/tcl-23-smart-shopping-list/issues/10

Acceptance Criteria

Type of Changes

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

Updates

Before

If a user wants to determine if an item exists in the grocery list, their option is to visually scan the list.

filterBefore

After

User is provided a labeled input where they can type to search their existing grocery list for items that may or may not be in it. A Reset Text Field button exists to clear the input field.

https://user-images.githubusercontent.com/38255956/117242015-00b19780-adfa-11eb-8137-534d6c4aa853.mov

Testing Steps / QA Criteria

  1. Navigate to branch ar-jw-filter-list
  2. npm install (for luck!)
  3. npm start
  4. Once you are on localhost:3000, navigate to localhost:3000/list
  5. If you would like to use a currently existing token with a list, you can use apart havoc vega. You are welcome to create your own token and list for testing if you would prefer.
  6. If you chose to create your own token and list, make sure you have added a few items to your list. Including items that share varying quantities of characters in the same order will demonstrate the new functionality. Example: drinking straws, strawberries, blueberries
  7. In the text field, start typing something that is in your list. You should see the list items change to match what you have typed.
  8. When done typing, click Reset Text Field to clear the text input field.
github-actions[bot] commented 3 years ago

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

https://tcl-23-shopping-list--pr25-ar-jw-filter-list-x719cpe9.web.app

(expires Sat, 15 May 2021 14:51:30 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

zeeatwork commented 3 years ago

The app returns me to the list view after I add an item. Is that part of a legacy acceptance criteria? I think users might want to add multiple items a decide to enter list view when they're done adding. I mistakenly tried to add items using the search bar because of this user flow.

apdsrocha commented 3 years ago

The app returns me to the list view after I add an item. Is that part of a legacy acceptance criteria? I think users might want to add multiple items a decide to enter list view when they're done adding. I mistakenly tried to add items using the search bar because of this user flow.

Hi @zeeatwork! I see what you mean, and I totally agree, for users adding a new list it'd probably make more sense to go directly to the add item view. If everyone else agrees, that's an easy fix to implement :D

jssckbl commented 3 years ago

@zeeatwork @apdsrocha I agree, too, that it feels like the flow intuitively should go to /add-item rather than /list.

With that said, I remember in a previous week being advised to have the app return the user to the /list view after successfully adding a new item. I know I supported changing the routing, and I believe I am the one who had to change it back :) I seem to recall it was @yenly that advised us to continue to implement the AC, and changing the route went against the AC. (Please correct me if I am wrong, Yenly!)

The two sources I can see that address the page view after adding an item are the following:

@yenly, @skylerwshaw, @laurenyz, let me know if I am sharing this correctly!

yenly commented 3 years ago

@zeeatwork @apdsrocha @jssckbl Great discussion on the interaction flow! 👏🏽

Yes, it's a good idea to implement according to AC and not to change functionalities from previous ACs. Based on your discussion, it sounds like this would be a good opportunity to create a new issue (or card) on our project board under Refactor and Misc Opportunities. Some possible labels could be feature-enhancement, nice-to-have, or refactor. This way, your ideas/discussion will be captured for later consideration when you have more time.

apdsrocha commented 3 years ago

When the field has text in it, the user should be able to tap a UI element (e.g., with an "X" button next to the field) to clear the field

Based on above AC, the "Reset Text Field" button should read "X". Does the above AC imply that if there's no text in the input field, should there be a reset button?

Hi @yenly, thank you for the feedback! In our understanding it wasn't a requirement that the UI element should only appear if there is text in the input. But we can conditionally render it, if that's's the case. As for the "X", it said "e.g.", so we read it as an example of what the element could say and not particularly a requirement. I personally feel like the UI element and it's content could be altered in later sprints when we make decisions about the styling and visual changes.

jssckbl commented 3 years ago

I love addressing the interpretations of the AC because my brain usually goes to extreme literal.

When the field has text in it, the user should be able to tap a UI element (e.g., with an "X" button next to the field) to clear the field

In the way this is written, I see it as only addressing what should be seen when the field as text in it, that a UI element is available. It definitely doesn't read to me that the UI element should not be visible otherwise. Also, having the UI element not exist if there is no text in the field feels like what I perceive would be an over-extension of what the developer is allowed to do, according to the AC.

And yes, the e.g. is something that I read as an example of many possibilities, and not a "do it just like this" instruction.

As @apdsrocha mentioned, these are absolutely things we can adjust if we both misinterpreted the AC! And maybe this is an instance where interpreting AC is a skill a developer builds, just like writing code or anything else :)

Let us know what you think, @yenly!