(feat) O3-2177: Tweak the patient banner details section to match designs #1197

Jexsie · 2023-06-07T19:56:36Z

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

Updates the patient banner to fit the designs
Fixes the relationships resource error
Fixes issue of not displaying address keys in the address column inside the patient banner
Updates configuration of the customAddressLabels

Screenshots

Screencasts

Screencast.from.2023-06-15.12-19-21.webm

Related Issue

https://issues.openmrs.org/projects/O3/issues/O3-2177

Other

Might require checking on #1175
Sample Config

{
  "@openmrs/esm-patient-banner-app": {
    "useCustomAddressLabel": {
      "enabled": true,
      "customAddressLabel": {
        "address1":"New Adress1 name",
        "district": "County",
        "address4": "Ward",
        "state": "Sub county",
        "city": "Village",
        "postalCode": "Postal Address",
        "country": "Location",
        "address5": "Sub Location",
        "address2": "Landmark"
      }
    }
  }
}

vasharma05 · 2023-06-08T05:44:00Z

Please add screenshots to get the review started.
Thanks!

vasharma05 · 2023-06-08T05:51:33Z

This is not specific to just tablet and desktop. The width of the patient banner will decide this.
For eg. on the advance patient search, the banner’s width is less and hence it will require 2 columns instead of 4.
This is implemented on the patient search, but will also include places where banner is being used in the workspaces.

vasharma05

Some nitpicking

Great work altogether 👏

packages/esm-patient-banner-app/src/banner/patient-banner.component.tsx

vasharma05 · 2023-06-15T11:11:56Z

packages/esm-patient-banner-app/src/banner/patient-banner.component.tsx

+    const currentRef = patientBannerRef.current;
+    const resizeObserver = new ResizeObserver((entries) => {
+      for (const entry of entries) {
+        setIsPatientBannerSmallSize(entry.contentRect.width < 1023);


vasharma05 · 2023-06-15T11:12:50Z

packages/esm-patient-banner-app/src/banner/patient-banner.test.tsx

@@ -60,6 +76,7 @@ describe('PatientBanner: ', () => {
    const user = userEvent.setup();

    const patientBannerSeachPageProps = { ...testProps, onClick: mockNavigateTo };
+    window.ResizeObserver = ResizeObserverMock;


Can you try adding the test to check that on decreasing the window size, the count of columns is changing or not.

I have catered for this in the Contact details component.

vasharma05 · 2023-06-15T11:14:31Z

packages/esm-patient-banner-app/src/config-schema.ts

+    enabled: {
+      _type: Type.Boolean,
+      _description: 'whether to enable using custom address labels',
+      _default: false,
+    },
+    customAddressLabel: {
+      _type: Type.Object,
+      _description: 'custom labels for addresses',
+      _default: {},
+    },


Why this change?
This change might break the other implementation.

Changing the config should be announced in the PR's description, as this will affect other people using the old config.

This is to fix the issue of not displaying the address keys.
We were using customAddressLabel object to conditionally the custom labels here
There is a solution, using

Object.keys(customAddressLabel).length > 0 ? ... : ...

but I thought adding the enabled boolean might be more explicit

@vasharma05 @denniskigen Please let's be a little more careful defining config schema, since as you say Vineet, changing things can affect existing implementations.

Having a boolean like this is redundant and not useful. Put yourself in the implementer's shoes and think about how they build a config. The only thing a boolean like this does is adds confusion and complexity, breaking things that should work.

Also, the way this has been implemented, the implementer has to provide every piece of the address as a key to the object or else it silently screws up (displays nothing where there should be a label). A better solution is to make it so an implementer only needs to provide configuration keys for the address labels they want to change.

But actually, I don't see why we don't just handle this using translation overrides? These strings go through translation. I suspect this isn't needed at all.

vasharma05 · 2023-06-15T11:15:41Z

packages/esm-patient-banner-app/src/contact-details/contact-details.component.tsx

+  const extractName = (display: string) => {
+    const pattern = /-\s*(.*)$/;
+    const match = display.match(pattern);
+    if (match && match.length > 1) {
+      return match[1].trim();
+    }
+    return display.trim();
+  };
+


@dkayiwa , please review this.
Thanks!

If the display property contains an identifier and a name, such as "100304E - John Doe", the regular expression pattern /-\s*(.*)$/ will match the hyphen and any subsequent whitespace characters. It captures the remaining part of the string (the name) using the (.*) group. The extracted name is then returned by the extractName function.

On the other hand, if the display property doesn't contain a hyphen and identifier, such as "John Doe", the regular expression pattern won't find a match. In that case, the extractName function will simply return the original display property string, trimmed of any leading or trailing whitespace.

Let me hope we won't have the display being "John Doe - 10008E", there it will break

packages/esm-patient-banner-app/src/contact-details/contact-details.component.tsx

packages/esm-patient-banner-app/src/contact-details/relationships.resource.tsx

packages/esm-patient-banner-app/src/hooks/usePatientListsForPatient.tsx

denniskigen · 2023-06-15T11:54:46Z

@donaldkibet @CynthiaKamau @makombe do you anticipate that these changes might break anything in your current config?

Jexsie · 2023-06-15T17:40:39Z

packages/esm-patient-banner-app/src/contact-details/contact-details.test.tsx

 describe('ContactDetails', () => {
-  it('renders an empty state view when relationships data is not available', async () => {


I removed this case since it was catered for in the empty view case for all sections

denniskigen

LGTM. Thanks, @Jexsie.

vasharma05 changed the title ~~(fix) Update the Patient Banner to fit the designs~~ (fix)O3-2177: Update the Patient Banner's show more details sections to match the designs Jun 8, 2023

vasharma05 changed the title ~~(fix)O3-2177: Update the Patient Banner's show more details sections to match the designs~~ (feat) O3-2177: Update the Patient Banner's show more details sections to match the designs Jun 8, 2023

Jexsie marked this pull request as ready for review June 13, 2023 18:48

vasharma05 reviewed Jun 15, 2023

View reviewed changes

Jexsie commented Jun 15, 2023

View reviewed changes

Jexsie force-pushed the fix/relation branch from 9295d6e to 7d7151a Compare June 15, 2023 17:43

Jexsie requested a review from vasharma05 June 15, 2023 18:12

denniskigen force-pushed the fix/relation branch from 80627ab to 22b4768 Compare June 21, 2023 21:02

denniskigen changed the title ~~(feat) O3-2177: Update the Patient Banner's show more details sections to match the designs~~ (feat) O3-2177: Tweak the patient banner details section to match designs Jun 21, 2023

Jexsie and others added 18 commits June 30, 2023 21:31

fix relationship resource

8db35e4

add patient list

f949d8e

banner updates

e565b4c

retouch design

1907ce3

tweak tests

0e81922

update patient list

f60285a

add empty state

492247d

fix eslint issue

3c6d150

update address configuration

0ae559c

retouch patient lists

0cdc8e5

extract name

e918291

update look on deacesed patient

6b9d61f

tweak tests

7ebfaef

clean up

cdac4ca

empty retrigger

cee877a

code review

12a8c03

make <p> tag <li> in patient list

00eb582

Tweaks

2e1ea3a

denniskigen force-pushed the fix/relation branch from f7e3119 to 2e1ea3a Compare June 30, 2023 18:37

denniskigen approved these changes Jun 30, 2023

View reviewed changes

denniskigen merged commit ea17f83 into openmrs:main Jun 30, 2023
5 checks passed

brandones mentioned this pull request Dec 27, 2023

BREAKING: Clean up patient banner config #1555

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(feat) O3-2177: Tweak the patient banner details section to match designs #1197

(feat) O3-2177: Tweak the patient banner details section to match designs #1197

Jexsie commented Jun 7, 2023 •

edited

vasharma05 commented Jun 8, 2023

vasharma05 commented Jun 8, 2023

vasharma05 left a comment •

edited

vasharma05 Jun 15, 2023

vasharma05 Jun 15, 2023

Jexsie Jun 15, 2023

vasharma05 Jun 15, 2023

Jexsie Jun 15, 2023 •

edited

brandones Dec 27, 2023 •

edited

vasharma05 Jun 15, 2023

Jexsie Jun 15, 2023 •

edited

denniskigen commented Jun 15, 2023

Jexsie Jun 15, 2023 •

edited

denniskigen left a comment

		describe('ContactDetails', () => {
		it('renders an empty state view when relationships data is not available', async () => {

(feat) O3-2177: Tweak the patient banner details section to match designs #1197

(feat) O3-2177: Tweak the patient banner details section to match designs #1197

Conversation

Jexsie commented Jun 7, 2023 • edited

Requirements

Summary

Screenshots

Screencasts

Related Issue

Other

vasharma05 commented Jun 8, 2023

vasharma05 commented Jun 8, 2023

vasharma05 left a comment • edited

Choose a reason for hiding this comment

vasharma05 Jun 15, 2023

Choose a reason for hiding this comment

vasharma05 Jun 15, 2023

Choose a reason for hiding this comment

Jexsie Jun 15, 2023

Choose a reason for hiding this comment

vasharma05 Jun 15, 2023

Choose a reason for hiding this comment

Jexsie Jun 15, 2023 • edited

Choose a reason for hiding this comment

brandones Dec 27, 2023 • edited

Choose a reason for hiding this comment

vasharma05 Jun 15, 2023

Choose a reason for hiding this comment

Jexsie Jun 15, 2023 • edited

Choose a reason for hiding this comment

denniskigen commented Jun 15, 2023

Jexsie Jun 15, 2023 • edited

Choose a reason for hiding this comment

denniskigen left a comment

Choose a reason for hiding this comment

Jexsie commented Jun 7, 2023 •

edited

vasharma05 left a comment •

edited

Jexsie Jun 15, 2023 •

edited

brandones Dec 27, 2023 •

edited

Jexsie Jun 15, 2023 •

edited

Jexsie Jun 15, 2023 •

edited