search

futil-js / contexture-react

React components for building contexture interfaces
https://smartprocure.github.io/contexture-react
MIT License
6 stars 4 forks source link

Feature/search data testid #537

Closed nico-olivares closed 2 years ago

nico-olivares commented 2 years ago

-Added two data-testid. One to the main search bar and one to the main search button. -Added three data-path. Facet, date, and number. (per discussion with Alejandro) -Updated the package.json (version number) -Updated the package-lock.json (version number) -Updated the changelog.mdx

decrapifier commented 2 years ago
Warnings
:warning: The README has not been updated. Please update the README.
Messages
:book: Could not find any browser results.

Generated by :no_entry_sign: dangerJS against 3c05f784178fcd5cf1af6cf97ac1b38047b78b1c

nico-olivares commented 2 years ago

If we're adding data-path to some example types we should add it to all the example types components to be consistent. Otherwise we'll have to make follow-up PRs to allow selecting other types of filters.

@stellarhoof I agree, but when I did something like that before I got push back for doing too many at once. Don't you think it would make sense to add a data-path to any component that receives node?

stellarhoof commented 2 years ago

Don't you think it would make sense to add a data-path to any component that receives node

Not necessarily. There are components that have access to node that are not "example types". Do not worry about the word "example". "Type" refers to the type of a node (tagsQuery, facet, number, etc...). Each type has a component so I don't think adding a data-path to each example type is overreaching because we know there is simply no good way of selecting filters. And we definitely have a use case for selecting filters in the tests.

TagsQuerySearchBar in particular is not actually a component for a node type but I think it also makes sense to add a data-path because there's no good way of selecting it right now.

nico-olivares commented 2 years ago

@stellarhoof take a second look. I got rid of the data-testids and added a single data-path to the whole component like you suggested. I also added a couple of other data-paths to what I saw made sense. Let me know if there's any I should get rid of or add.

nico-olivares commented 2 years ago

@daedalus28 could you please review / merge? I don't know who else has merging permits here. Thanks,