-
Notifications
You must be signed in to change notification settings - Fork 47
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
Refactor: Relationships UI improvements #278
Refactor: Relationships UI improvements #278
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @amosmachora LGTM. There are some suggestions that you can address
...s/esm-patient-clinical-view-app/src/family-partner-history/family-relationship.workspace.tsx
Outdated
Show resolved
Hide resolved
...s/esm-patient-clinical-view-app/src/family-partner-history/family-relationship.workspace.tsx
Outdated
Show resolved
Hide resolved
...s/esm-patient-clinical-view-app/src/family-partner-history/family-relationship.workspace.tsx
Outdated
Show resolved
Hide resolved
...s/esm-patient-clinical-view-app/src/family-partner-history/family-relationship.workspace.tsx
Outdated
Show resolved
Hide resolved
...s/esm-patient-clinical-view-app/src/family-partner-history/family-relationship.workspace.tsx
Outdated
Show resolved
Hide resolved
...s/esm-patient-clinical-view-app/src/family-partner-history/family-relationship.workspace.tsx
Outdated
Show resolved
Hide resolved
...s/esm-patient-clinical-view-app/src/family-partner-history/family-relationship.workspace.tsx
Outdated
Show resolved
Hide resolved
packages/esm-patient-clinical-view-app/src/family-partner-history/relationships.resource.tsx
Outdated
Show resolved
Hide resolved
packages/esm-patient-clinical-view-app/src/family-partner-history/family-relationship.scss
Show resolved
Hide resolved
|
||
.buttonSet { | ||
margin-top: auto; | ||
display: flex; | ||
} | ||
|
||
.button { | ||
height: spacing.$spacing-10; | ||
display: flex; | ||
align-content: flex-start; | ||
align-items: center; | ||
max-width: 50%; | ||
min-width: 50%; | ||
width: 50%; | ||
} | ||
|
||
.grid { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try to localize the naming to avoid style overrides.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay got it
@makombe thanks for the feedback. I will work on them and update the PR accordingly. |
…ory/family-relationship.workspace.tsx Co-authored-by: Makombe Kennedy <[email protected]>
Co-authored-by: Makombe Kennedy <[email protected]>
@ojwanganto I have added the other relationships tab. I have also added a workspace to facilitate adding the same. @makombe there is a small issue that I have. I want to filter out the |
@@ -120,39 +120,39 @@ export const configSchema = { | |||
_default: [ | |||
{ | |||
uuid: '8d91a01c-c2cc-11de-8d13-0010c6dffd0f', | |||
displayAIsToB: 'Sibling', | |||
display: 'Sibling/Sibling', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1. The config looks good
Thanks, @amosmachora. It's looking good. Tagging @makombe and @donaldkibet for in-depth reviews. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks @amosmachora
return ( | ||
<DataTableSkeleton | ||
headers={headers} | ||
aria-label="patient bills table" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this miss placed label?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah this is misplaced let me fix this.
@makombe i fixed the misplaced aria label and that by proxy markd your approval as stale. Mind approving again ? |
Thanks, @amosmachora. |
@amosmachora Please resolve the conflicts then we can merge it in. I think it LGMT |
Requirements
Summary
In thi PR I have updated the UI for the relationships page on the patient chart. Previously when adding a new relationship the user was being redirect to a new page. In this PR I have replaced that functionality with a workspace which allows the user to remain in a single page improving the ux drastically.
Also I made some small changes on the labels to more accurately describing what action the button will be persorming e.g
Add PNS Contact
.There is a small UI bug when searching for a patient that makes the results appear terrible. I will address that in a different PR. All feedback will be highly appreciated.
Screenshots
Screenshot A
Screenshot B
Other Relationships Tab Screenshot
Video
Untitled.video.-.Made.with.Clipchamp.mp4
Related Issue
None.
Other
None.