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

(fix) O3-3589: Enable data table to search based on all table columns #76

Merged
merged 2 commits into from
Jul 19, 2024

Conversation

arodidev
Copy link
Contributor

Requirements

  • This PR has a title that briefly describes the work done including the ticket number. If there is a ticket, make sure your PR title includes a conventional commit label. See existing PR titles for inspiration.
  • My work conforms to the OpenMRS 3.0 Styleguide and design documentation.
  • My work includes tests or is validated by existing tests.

Summary

This PR fixes an issue with the data table search bar whereby the search was being made based on specific column. This fix introduces a more inclusive search that targets all the columns in the table.

Screenshots

Screen.Recording.2024-07-16.at.07.17.29.mov

Related Issue

https://openmrs.atlassian.net/browse/O3-3589

Other

@arodidev arodidev marked this pull request as ready for review July 16, 2024 05:33
@arodidev arodidev requested review from donaldkibet, samuelmale and CynthiaKamau and removed request for donaldkibet July 16, 2024 05:33
Copy link
Contributor

@CynthiaKamau CynthiaKamau left a comment

Choose a reason for hiding this comment

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

Thanks @arodidev , there seems to be a preference to using debounce while doing searches. Is it something you would consider?

@arodidev
Copy link
Contributor Author

I'll work on adding the same, thanks @CynthiaKamau

};
});

if (searchString && searchString.trim() !== "") {
Copy link
Member

Choose a reason for hiding this comment

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

You want to do this soft check before the mapping above so that we return early incases of an empty search term.

orderer: string;
}

function useSearchResults(tableEntries: Order[], searchString): CustomOrder[] {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
function useSearchResults(tableEntries: Order[], searchString): CustomOrder[] {
function useOrdersSearchResults(orders: Order[], searchTerm): CustomOrder[] {

Can you define this hook in a separate file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, forgot to relocate this

Comment on lines 56 to 65
const flattenedData = tableEntries.map((eachObject) => {
return {
...eachObject,
dateActivated: formatDate(parseDate(eachObject.dateActivated)),
patient: eachObject.patient?.display.split("-")[1],
patientUuid: eachObject.patient?.uuid,
status: eachObject.fulfillerStatus ?? "--",
orderer: eachObject.orderer?.display.split("-")[1],
};
});
Copy link
Member

@samuelmale samuelmale Jul 16, 2024

Choose a reason for hiding this comment

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

Should we be doing this mapping on every search event?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll memoize this and pass it as an argument.

@arodidev arodidev merged commit 7f0ce7c into openmrs:main Jul 19, 2024
3 checks passed
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