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 UI-select-extended searchable issues #166

Merged
merged 2 commits into from
Feb 5, 2024
Merged

Conversation

kajambiya
Copy link
Collaborator

@kajambiya kajambiya commented Feb 1, 2024

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

The UI-select-extended control has 2 flavours:
Preloaded(default): Where the data source is loaded on page load and you just search through the items.
Searchable: Data source is not pre loaded but as you type, a request is made to the endpoint to return results matching the search criteria.
There were bugs for the searchable flavours, i.e in edit mode, it would not pick the saved value. Also in enter mode, it would not show the option for reusing previous value.
This PR seeks to resolve the issues above

Screenshots

Screen.Recording.2024-02-01.at.13.57.52.mov
Screenshot 2024-02-01 at 13 56 35

Related Issue

Other

@pirupius
Copy link
Member

pirupius commented Feb 2, 2024

@kajambiya was the intention of the video on the description meant to show the searchable demo?

@kajambiya
Copy link
Collaborator Author

@kajambiya was the intention of the video on the description meant to show the searchable demo?

It was to show that the searchable Ui-Select-Extended control picks a value in edit mode.

Comment on lines 33 to 34
const [savedSearchableItemDisplay, setSavedSearchableItemDisplay] = useState('');
const [savedSearchableItem, setSavedSearchableItem] = useState({});
Copy link
Member

Choose a reason for hiding this comment

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

These 2 states might become confusing. Isn't savedSearchableItemDisplay derived from savedSearchableItem?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nice catch. Fixed

Comment on lines 73 to 74
setSavedSearchableItem(dataItem);
setSavedSearchableItemDisplay(dataItem.display);
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason why these 2 are separated? it looks like duplication

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

src/datasources/location-data-source.ts Show resolved Hide resolved
@pirupius pirupius merged commit 6124a7c into openmrs:main Feb 5, 2024
5 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.

2 participants