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

[DUOS-2901] Add search bar to libraries #2456

Merged
merged 13 commits into from
Mar 13, 2024

Conversation

aarohinadkarni
Copy link
Contributor

@aarohinadkarni aarohinadkarni commented Feb 7, 2024

Addresses

https://broadworkbench.atlassian.net/browse/DUOS-2901

Summary

Adds search bars to our two data libraries: Researcher Console, Data Library and SO Console, My Datasets.


Have you read Terra's Contributing Guide lately? If not, do that first.

  • Label PR with a Jira ticket number and include a link to the ticket
  • Label PR with a security risk modifier [no, low, medium, high]
  • PR describes scope of changes
  • Get a minimum of one thumbs worth of review, preferably two if enough team members are available
  • Get PO sign-off for all non-trivial UI or workflow changes
  • Verify all tests go green
  • Test this change deployed correctly and works on dev environment after deployment

@aarohinadkarni aarohinadkarni changed the title An duos 2901 add search bar to libraries [DUOS-2901] Add search bar to libraries Feb 7, 2024
@aarohinadkarni aarohinadkarni marked this pull request as ready for review February 8, 2024 17:11
@aarohinadkarni aarohinadkarni requested a review from a team as a code owner February 8, 2024 17:11
@rushtong
Copy link
Contributor

rushtong commented Feb 8, 2024

One detail I'm seeing is that the search bar and the access type filters are over-writing the results without changing the filter values. For example, enter a value in the search bar and it will filter records. Then check one of the access type checkboxes, and the search bar filter is no longer in effect, yet the value still shows in the search bar. A similar effect happens in reverse. I think we need to take into account BOTH the search bar value and the access type checkboxes in the search handler function.

Copy link
Contributor

@rushtong rushtong left a comment

Choose a reason for hiding this comment

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

I'm very impressed with this implementation, thank you! 👍🏽

Copy link
Contributor

@fboulnois fboulnois 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 good, a few minor suggestions:

src/components/data_search/DatasetSearchTable.js Outdated Show resolved Hide resolved
src/components/data_search/DatasetSearchTable.js Outdated Show resolved Hide resolved
src/components/data_search/DatasetSearchTable.js Outdated Show resolved Hide resolved
src/components/data_search/DatasetSearchTable.js Outdated Show resolved Hide resolved
Copy link
Contributor

@fboulnois fboulnois 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 👍

@aarohinadkarni aarohinadkarni merged commit 51da910 into develop Mar 13, 2024
9 checks passed
@aarohinadkarni aarohinadkarni deleted the an-DUOS-2901-Add-Search-Bar-to-Libraries branch March 13, 2024 14:23
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.

3 participants