-
Notifications
You must be signed in to change notification settings - Fork 241
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
(feat) O3-3246 - Ward App - open patient record in workspace #1226
Conversation
Size Change: -5.77 kB (-0.16%) Total Size: 3.51 MB
ℹ️ View Unchanged
|
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.
Basically LGTM, a few minor comments / questions.
onClick={() => | ||
launchWorkspace<WardPatientWorkspaceProps>('ward-patient-workspace', { patientUuid: props.patient.uuid }) | ||
}> | ||
{/* Name will not be displayed; just there for a11y */} |
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.
Not sure what this means, but ok
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.
a11y = accessibility. The name serves as a label for the box/button so that the browser knows what it is.
@@ -0,0 +1,24 @@ | |||
import React from 'react'; |
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.
What is this component for? Is it related to this ticket?
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.
Oh yeah, stubbed that out before I realized it wasn't asked for in the ticket. Removing it.
@@ -0,0 +1,18 @@ | |||
import React from 'react'; |
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.
It is unclear if or where this is used here. Is this related to this feature?
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.
Things with the .extension.tsx
suffix are extensions, which are used in the index file. If something has the extension suffix and isn't used in the index file, then something has gone wrong.
|
||
useEffect(() => { | ||
if (isLoading) { | ||
setTitle('Ward Patient', <InlineLoading />); |
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.
This should probably be translated, no? And for the error case below too.
Requirements
Summary
The skeleton of a patient workspace in the ward app.
Screenshots
I don't know what's going on with the patient info in the cards but it's unrelated to this ticket.
recording-2024-07-03_15.25.13.mp4
Related Issue
https://openmrs.atlassian.net/browse/O3-3246
Other