Skip to content
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

7. add search list form and function in List #25

Merged
merged 3 commits into from
Feb 23, 2024
Merged

7. add search list form and function in List #25

merged 3 commits into from
Feb 23, 2024

Conversation

hun-ah
Copy link
Collaborator

@hun-ah hun-ah commented Feb 21, 2024

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

Description

  • added form element to the List component
  • the form contains an input that allows the user to search for an item on their list (case insensitive and can search any part of the item name)
  • added clear functionality to clear input

Related Issue

closes #7

Acceptance Criteria

  • A form is added to the top of the List view, above the shopping list
  • The form includes the following elements
    • A text field (with semantic <label>!) which narrows down the list as the user types
    • When there’s text in the field, some kind of button (e.g., an X) to clear the field. When the field is cleared, the list is reset to its unfiltered state.

Type of Changes

Enchancement

Updates

Before

Screenshot 2024-02-21 at 1 59 03 PM

After

Screen.Recording.2024-02-21.at.1.59.42.PM.mov

Testing Steps / QA Criteria

  1. git pull
  2. git checkout ec-hm-issue-7
  3. npm start
  4. select list from home
  5. go to list page and search for a list item
  6. clear input text using the 'X' button

@hun-ah hun-ah added the enhancement New feature or request label Feb 21, 2024
Copy link

github-actions bot commented Feb 21, 2024

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

https://tcl-66-smart-shopping-list--pr25-ec-hm-issue-7-eu7cc3wm.web.app

(expires Fri, 01 Mar 2024 16:18:37 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 2dc16cb2a79cbd6723fdc511b73e0743df1d18d0

@eonflower
Copy link
Collaborator

Everything seems to work correctly except when I searched for one specific item. I used the groceries list that we all have since it had the most items added to it. But when I try and search for banana, it doesn't pop up. I tried using a capital B since the original list item is "Banana", but that also didn't do anything. If I search 'an' it pops up though. I added some more items that start with a capital and it seems they also have that issue.

@hun-ah hun-ah changed the title add search list form and function in List 7. add search list form and function in List Feb 21, 2024
@hun-ah
Copy link
Collaborator Author

hun-ah commented Feb 22, 2024

Good call @eonflower. So there are two ways this can be fixed, the first is just to do this item.name.toLowerCase().includes(search.toLowerCase()) on the filtered data or alternatively we would add a .toLowerCase() on any item being sent to the database, which I think is the better option to ensure that all the data sent to firebase is consistent and could prevent any future errors. If everything is being stored differently ie (apple, BANANA, Mango) it makes the data harder to work with vs. if it were (apple, banana, mango) or whatever the list item is. @StefieCaff let me know your thoughts as well and I will make the change!

*note: if we change the items going into the database we will also have to edit any existing items to be consistent with being in all lower case but it's a relatively easy change.

@eonflower
Copy link
Collaborator

Personally, I like consistency within the database and then we can style the items to have the first letter capitalized later if we want that.

@StefieCaff
Copy link
Collaborator

StefieCaff commented Feb 22, 2024

@hun-ah , I agree with the consensus. Mange the format of the data before it enters the database. I did notice in the database that the item may actually be stored by a random number... there is a name key that stores the input value... but if I am seeing it correctly the items may be assigned a random number just wanted to bring that up :)

Copy link
Collaborator

@StefieCaff StefieCaff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! Super clean code-- I noticed when I did a search with just a space, it worked to filter any items that had a space in them, and I liked that.

@3campos
Copy link
Collaborator

3campos commented Feb 22, 2024

@eonflower @StefieCaff @hun-ah Per our consensus, the "toLowerCase" method has been added to firebase.js to apply a lowercase format to the grocery item names that a user creates. I edited all preexisting grocery names in our firestore database which contained capital letters to be in lowercase format.

Copy link
Collaborator

@eonflower eonflower left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job y'all!

Copy link
Collaborator

@jeremiahfallin jeremiahfallin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work, looks good to me!

@@ -1,13 +1,43 @@
import { ListItem } from '../components/ListItem';
import { useState } from 'react';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extremely pedantic but it's common practice to put library imports at the very top above files/components that are in the project.

onChange={handleChange}
value={search}
/>
<button onClick={handleClear}>x</button>
Copy link
Collaborator

@jeremiahfallin jeremiahfallin Feb 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also pedantic but this should have type="button" on it, even with the e.preventDefault since this button will technically try to submit the form.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback, @jeremiahfallin! We implemented both of your recommendations.

@3campos 3campos merged commit 9b54739 into main Feb 23, 2024
2 checks passed
@3campos 3campos deleted the ec-hm-issue-7 branch February 23, 2024 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7. As a user, I want to filter my shopping list to make it easier to locate an item in the list
5 participants