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

I86 search by pet names in contacts page #88

Merged
merged 20 commits into from Jul 21, 2022

Conversation

swolrus
Copy link
Collaborator

@swolrus swolrus commented Jul 17, 2022

Change Summary

  • Searches by people and pets
  • Updated the pet section so that it's a single text field
  • Mock data needed to be updated

Related Issue

@vercel
Copy link

vercel bot commented Jul 17, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
poops ✅ Ready (Inspect) Visit Preview Jul 20, 2022 at 3:20PM (UTC)

@DaveA-W
Copy link

DaveA-W commented Jul 18, 2022

Hi team, just a note - 9/10 clients will only have one pet.
As such, suggest "Pet(s)" really only needs to be a single text box - e.g. "Fido" or "Frank & Ginger". e.g. you can see that we capture the source data as a single "Name(s)" field at https://www.poopswa.org.au/client-application/
image

So it's probably not necessary to normalise pet(s) into separate entities with independent notes - per what I've seen in one of the PR previews below :)

Only loosely related to this PR, but has implications for ease of display/searching - so just the heads up!

Cheers, Dave.

image

@ChantelleKerr
Copy link
Contributor

ChantelleKerr commented Jul 18, 2022

Hi team, just a note - 9/10 clients will only have one pet. As such, suggest "Pet(s)" really only needs to be a single text box - e.g. "Fido" or "Frank & Ginger".
So it's probably not necessary to normalise pet(s) into separate entities with independent notes - per what I've seen in one of the PR previews below :)

Only loosely related to this PR, but has implications for ease of display/searching - so just the heads up!

Cheers, Dave.

Hi Dave, I've fixed the pet section.

@skyheat
Copy link
Member

skyheat commented Jul 18, 2022

Hi team, just a note - 9/10 clients will only have one pet. As such, suggest "Pet(s)" really only needs to be a single text box - e.g. "Fido" or "Frank & Ginger". e.g. you can see that we capture the source data as a single "Name(s)" field at https://www.poopswa.org.au/client-application/ image

So it's probably not necessary to normalise pet(s) into separate entities with independent notes - per what I've seen in one of the PR previews below :)

Only loosely related to this PR, but has implications for ease of display/searching - so just the heads up!

Cheers, Dave.

image

Hi Dave,

We initially had pets separated so that the user could choose which specific pet they were doing a visit for as well as selecting the specific pet for incident/vet reports.

However, we would be happy to go with the way you've suggested, making the user select the client(s) and inputting the specific pet(s). Vet/Incident reports would operate the same way, with the user manually inputting the pet's name.

@DaveA-W
Copy link

DaveA-W commented Jul 18, 2022 via email

Copy link
Collaborator

@chenga3 chenga3 left a comment

Choose a reason for hiding this comment

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

Overall looks really good. A few suggestions:

  • Search bar currently searches in the middle of words too, might be better to only pattern match from the beginning of words (this might be a big task so maybe merge this PR before tackling this if you choose to)?
  • Convention is to use PascalCase for component file names (e.g. SearchTag.tsx rather than searchtag.tsx)
  • searchbar.tsx could be renamed to index.tsx if the SearchBar folder represents that component

src/pages/contact/index.tsx Outdated Show resolved Hide resolved
NicholasChoong
NicholasChoong previously approved these changes Jul 20, 2022
Copy link
Member

@NicholasChoong NicholasChoong left a comment

Choose a reason for hiding this comment

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

looks good

@swolrus swolrus merged commit 5d3e098 into main Jul 21, 2022
@swolrus swolrus deleted the i86-search_by_pet_names_in_contacts_page branch July 21, 2022 05:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Search by pet names in Contacts page
6 participants