-
Notifications
You must be signed in to change notification settings - Fork 234
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-3250: Add in-patient notes workspace #1238
Conversation
f4522e8
to
7d24c56
Compare
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 generally looks good to me, but I am going to defer to @chibongho and @brandones who has been working with this code longer.
One thing I'm missing... where is the code that handles saving the notes?
Also fyi looks like there are some failing tests.
This is handled in the |
Ah, okay, so basically this rendered the existing visit note within the Ward context? |
I created a separate form for the ward in-patient note. |
81dfc38
to
372e66e
Compare
packages/esm-ward-app/src/ward-patient-card/ward-patient-card.tsx
Outdated
Show resolved
Hide resolved
fd340b4
to
0935272
Compare
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.
Generally looks great @usamaidrsk , but we should fetch the configuration from the "configuration" endpoint instead of setting up a new notes config, see my comments... sorry, I should have mentioned that before, didn't think of it.
I also flagged Ian and Dennis for a few years, but this is close, thanks, I'm excited to see this working!
import type { PatientCardElementType, WardPatientCardProps } from '../../types'; | ||
import styles from './style.scss'; | ||
|
||
const WardPatientWorkspaceBanner = ({ bed, patient, visit }: WardPatientCardProps) => { |
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.
Will defer to @chibongho when he gets back on this part...
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.
@chibongho I hope you can now take a look at this. Thanks
packages/esm-ward-app/src/ward-workspace/ward-patient-notes/notes-config-schema.ts
Outdated
Show resolved
Hide resolved
Oh, and also, looks like some merge errors... |
0935272
to
5e2854a
Compare
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.
@usamaidrsk I made a few small requests for changes, but generally LGTM, I approved.
Would be still be good to get a review from @chibongho Thanks!
packages/esm-ward-app/src/ward-patient-card/ward-patient-card.tsx
Outdated
Show resolved
Hide resolved
be62ffe
to
8b5656a
Compare
packages/esm-ward-app/src/ward-workspace/ward-patient-notes/form/notes-form.test.tsx
Outdated
Show resolved
Hide resolved
packages/esm-ward-app/src/ward-workspace/ward-patient-notes/form/notes-form.test.tsx
Outdated
Show resolved
Hide resolved
packages/esm-ward-app/src/ward-workspace/ward-patient-notes/notes-action-button.extension.tsx
Outdated
Show resolved
Hide resolved
packages/esm-ward-app/src/ward-workspace/ward-patient-notes/notes-action-button.extension.tsx
Outdated
Show resolved
Hide resolved
packages/esm-ward-app/src/ward-workspace/ward-patient-notes/notes.style.scss
Outdated
Show resolved
Hide resolved
Hi @usamaidrsk! @mogoodrich, I have some suggestions which should go in before the changes are merged. |
Thanks @vasharma05 ! Defer to you when this is ready to be merged. |
184dfb3
to
4c355f6
Compare
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.
A few more changes. Great work @usamaidrsk!
One thing I'm wondering, the notes form is in the same app as of the Extension slot where it is being rendered, why aren't we directly importing it into the workspace?
Cc: @mogoodrich @chibongho
packages/esm-ward-app/src/ward-patient-workspace/ward-patient.workspace.tsx
Outdated
Show resolved
Hide resolved
packages/esm-ward-app/src/ward-workspace/ward-patient-notes/form/notes-form.component.tsx
Outdated
Show resolved
Hide resolved
packages/esm-ward-app/src/ward-workspace/ward-patient-notes/form/notes-form.component.tsx
Outdated
Show resolved
Hide resolved
packages/esm-ward-app/src/ward-workspace/ward-patient-notes/form/notes-form.test.tsx
Outdated
Show resolved
Hide resolved
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 @usamaidrsk!
Thanks @usamaidrsk ! Just waiting for all the checks to pass, then I will merge. |
Requirements
Summary
This PR adds in-patient notes workspace
Screenshots
patient-note.mp4
Related Issue
Other
O3-3250