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

(feat) O3-2614: Move Allergen tabs to a single picker in allergy form #1525

Merged
merged 15 commits into from
Dec 15, 2023

Conversation

jayasanka-sack
Copy link
Member

@jayasanka-sack jayasanka-sack commented Dec 5, 2023

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 pull request aims to enhance the user experience of the allergy form by consolidating allergen tabs into a single, efficient picker. The modifications have been made as part of the epic O3-2561.

Key Changes:

  1. Introducing allergen-picker Component:
    A new component, allergen-picker, has been created to serve as a custom field using the React Hook Forms controller. This component takes charge of rendering the allergen picker and implementing the filter functionality. Refer to the component's tests for a comprehensive understanding of its behavior.
  2. Test Enhancements: The existing unit test suite has been expanded to ensure thorough coverage, addressing the issue of disabled and outdated tests. Additionally, unused values in the mock file, remnants from the pre-FHIR transition, have been removed for a cleaner codebase.
  3. Custom Allergen Warning Message: Users attempting to add a custom allergen will now encounter a warning message. The message emphasizes that introducing custom entries may impact system-wide allergy notifications. Users are encouraged to choose from the provided list for accurate alerts, as custom entries may not trigger notifications in all relevant contexts.

Screenshots

image image image

Related Issue

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

Other

Meanwhile, I've identified three bugs that warrant attention:
image

  1. Custom Allergens and Reactions Display as "Other":
    Both custom allergens and reactions consistently appear as "Other" regardless of the input provided. This anomaly is likely stemming from the FHIR module during the conversion of allergies to FHIR allergy objects. https://openmrs.atlassian.net/browse/O3-2634
  2. Inconsistent Severity Display as "UNABLE-TO-ASSESS":
    On certain occasions, the severity is observed to display as "UNABLE-TO-ASSESS." To determine if this behavior is intentional or unintentional, further investigation is needed.
    https://openmrs.atlassian.net/browse/O3-2633
  3. Submit button on Allergy Form remains disabled when filling allergen and severity before reactions:
    When filling out the Allergy Form on OpenMRS, there is a bug preventing the submit button from enabling properly. The issue arises when the allergen and severity fields are filled first, followed by selecting reactions. The save button remains disabled until another field is changed.
    https://openmrs.atlassian.net/browse/O3-2629

@jayasanka-sack jayasanka-sack marked this pull request as draft December 5, 2023 11:23
@jayasanka-sack jayasanka-sack changed the title WIP: Move Allergen tabs to a single picker in allergy form WIP: (feat) O3-2614: Move Allergen tabs to a single picker in allergy form Dec 6, 2023
@jayasanka-sack jayasanka-sack changed the title WIP: (feat) O3-2614: Move Allergen tabs to a single picker in allergy form (feat) O3-2614: Move Allergen tabs to a single picker in allergy form Dec 6, 2023
@jayasanka-sack jayasanka-sack marked this pull request as ready for review December 6, 2023 08:29
@jayasanka-sack
Copy link
Member Author

Update: the severity issue is now fixed. check 023bd8d and https://talk.openmrs.org/t/reassessing-allergy-severity-mapping-in-fhir/41083
image

@vasharma05
Copy link
Member

Hi @jayasanka-sack !
Great work 👏
Just a few quick points that I believe can be improved (from the video shared in the description):

  1. Instead of having a search component and then having a custom dropdown, can we instead use a carbon ComboBox, as it suits the needs and doesn't bring other UX challenges, which carbon handles (e.g. keyboard arrows for selecting an option).
  2. The buttons (Save and Discard) should be sticked at the end of the workspace, they are moving up and down, as the user is opening/ closing the allergies list.

Diving into the code now.
Thanks!

@denniskigen
Copy link
Member

denniskigen commented Dec 13, 2023

Thanks for the work, @jayasanka-sack. Is having the dropdown automatically open after launching the workspace the ideal UX? I found it a bit jarring.

allergies-dropdown.mp4

@ibacher
Copy link
Member

ibacher commented Dec 13, 2023

Is having the dropdown automatically open after launching the workspace the ideal UX? I found it a bit jarring.

I can see some argument for having that pre-open. However, style-wise, it seems like the the search box and the list of items should match in terms of font-size and background color. See the Carbon pattern docs on search.

Specifically, the examples I see there look like this:

Screenshot 1 Screenshot 2

@jayasanka-sack
Copy link
Member Author

Thanks for the reviews! Using the carbon ComboBox makes much sense. I'll update it and let you know.

@jayasanka-sack
Copy link
Member Author

Update:

  • Change the allergen picker to a Combo Box with: 7869870
  • Stick action buttons to the bottom of the form: 6250e92
image image image image

@jayasanka-sack
Copy link
Member Author

jayasanka-sack commented Dec 15, 2023

@vasharma05 @ibacher and @denniskigen I updated the PR. Would you mind taking a look when you have a moment?

Thanks! 🙌

Copy link
Member

@denniskigen denniskigen left a comment

Choose a reason for hiding this comment

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

Excellent work, @jayasanka-sack. Unfortunately, it looks like i18next-parser added zero width non-breaking space characters (\u200b) to some Khmer translations. I've taken the liberty of pushing a commit that fixes that.

Two possible next steps in the evolution of the allergies feature are:

  • Edit functionality for allergies
  • Making sure the allergies details tile in the Patient Header works in the tablet viewport

@denniskigen denniskigen merged commit 53ce61a into openmrs:main Dec 15, 2023
6 checks passed
@jayasanka-sack jayasanka-sack deleted the O3-2614 branch December 18, 2023 07:06
@jayasanka-sack jayasanka-sack restored the O3-2614 branch January 23, 2024 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants