Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

[WIP] feat(patient) secondary, tertiary addresses, phone numbers, and emails #2103

Closed

Conversation

sourabbanka22
Copy link
Contributor

A Feature that adds the ability to define secondary, tertiary, etc. phone numbers, addresses, and
emails for a patient.

New Feature #1920
Fixes #1920 .

Below are some Screenshots of the Added Functionality:
Before Adding PhoneNumber and Email of a Patient:
Before

After Adding PhoneNumber and Email:
After

User needs to click on the Add Button to add phone numbers, emails, etc to the respective array.

Currently I have commented out few tests related to Phone Number, Email and Address Models

Please let me know if there are any Modifications or Changes required.

A Feature that adds the ability to define secondary, tertiary, etc. phone numbers, addresses, and
emails for a patient.

New Feature HospitalRun#1920
@gitpod-io
Copy link

gitpod-io bot commented May 28, 2020

@vercel
Copy link

vercel bot commented May 28, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/hospitalrun/hospitalrun-frontend/ckm8w3x1l
✅ Preview: https://hospitalrun-fron-git-fork-sourabbanka22-secondary-tertia-873150.hospitalrun.now.sh

@jackcmeyer jackcmeyer changed the title New Feature: secondary, tertiary (etc) addresses, phone numbers, and emails feat(patient) secondary, tertiary (etc) addresses, phone numbers, and emails May 28, 2020
@jackcmeyer jackcmeyer marked this pull request as draft May 28, 2020 21:56
@jackcmeyer jackcmeyer self-requested a review May 28, 2020 21:56
Copy link
Member

@jackcmeyer jackcmeyer left a comment

Choose a reason for hiding this comment

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

Design changes:

Create Patient

  • When creating a new patient, I think that the everything should be inside the table rather than having a table only for the already added phone numbers. So, the page loads with the email, phone number, etc tables already loaded and there are empty boxes inside the table. (I hope that makes sense).
  • I think the button should be on the bottom right side of the table since that is the order a user would work in
  • I think the buttons should say "Add Email, "Add Phone Number", etc.

View Patient

  • When viewing a patient, there should be no add buttons. Only the tables should be seen here.

Both

  • I think the tables should render inside a panel that better distinguishes what the table means. The heading of the panel should be "Address", "Email", etc.

Design Changes for Phone Number, Email and Address Model

fix HospitalRun#1920
@sourabbanka22
Copy link
Contributor Author

@jackcmeyer I have made the design changes that you suggested

Please let me know if there are any Modifications or Changes required.

@matteovivona matteovivona changed the title feat(patient) secondary, tertiary (etc) addresses, phone numbers, and emails feat(patient) secondary, tertiary addresses, phone numbers, and emails May 29, 2020
@matteovivona matteovivona added the in progress indicates that issue/pull request is currently being worked on label May 29, 2020
@matteovivona matteovivona added this to the v2.0 milestone May 29, 2020
@matteovivona matteovivona marked this pull request as ready for review May 29, 2020 21:14
Copy link
Member

@jackcmeyer jackcmeyer left a comment

Choose a reason for hiding this comment

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

Some more design changes:

  • I think the button format should be in the link styling to be a little smaller.
  • The button should be outside of the table in the of the table, that way the table content takes up the whole panel.

@jackcmeyer jackcmeyer requested review from matteovivona, fox1t and a user May 30, 2020 04:16
Copy link
Member

@jackcmeyer jackcmeyer left a comment

Choose a reason for hiding this comment

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

Sorry for the number of review comments, mostly around:

  • duplicate code
  • internationalization of keys
  • we'll need to get tests added for the new functionality, plus update any existing tests

Please let me know anything I can do to help!

src/patients/patient-slice.ts Outdated Show resolved Hide resolved
src/model/ContactInformation.ts Outdated Show resolved Hide resolved
src/patients/patient-slice.ts Outdated Show resolved Hide resolved
src/patients/patient-slice.ts Outdated Show resolved Hide resolved
src/patients/GeneralInformation.tsx Outdated Show resolved Hide resolved
src/patients/GeneralInformation.tsx Outdated Show resolved Hide resolved
src/patients/GeneralInformation.tsx Outdated Show resolved Hide resolved
src/patients/GeneralInformation.tsx Outdated Show resolved Hide resolved
src/patients/GeneralInformation.tsx Outdated Show resolved Hide resolved
@vercel vercel bot temporarily deployed to Preview May 30, 2020 08:11 Inactive
Copy link
Contributor

@matteovivona matteovivona left a comment

Choose a reason for hiding this comment

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

@sourabbanka22 tests have dropped too far, exceeding our maximum limit to accept a PR. I'd ask you to check it out and fix it.

@sourabbanka22
Copy link
Contributor Author

sourabbanka22 commented May 30, 2020

@tehkapa I am working on fixing the tests

Component Instances for Phone Numbers, Addresses and Emails

re HospitalRun#1920
@sourabbanka22
Copy link
Contributor Author

Hi @kumikokashii , I have created component instances for Phone Numbers, Emails and Addresses, does the code look good to you?

@tehkapa I have added all the required tests for the Added Functionalities, please let me know if there is anything else that I can help with

@matteovivona matteovivona requested review from jackcmeyer, matteovivona and a user June 8, 2020 12:59
@ghost
Copy link

ghost commented Jun 8, 2020

@sourabbanka22 Thank you for the updates 🙏 It's looking almost ready to go. I'm gonna jump in and tweak it a bit. Please stay tuned. Thanks again!

@ghost ghost changed the title feat(patient) secondary, tertiary addresses, phone numbers, and emails [WIP] feat(patient) secondary, tertiary addresses, phone numbers, and emails Jun 10, 2020
@ghost ghost marked this pull request as draft June 10, 2020 04:17
@ghost ghost mentioned this pull request Jun 10, 2020
@ghost
Copy link

ghost commented Jun 12, 2020

In addition to allowing more than one phone number, etc, updating the GeneralInformation component to have its onChange prop to pass the entire patient instead of a key and a value. Due to high volume of changes, transitioning to another branch. Please see #2113.

@ghost ghost closed this Jun 12, 2020
@ghost ghost removed help wanted indicates that an issue is open for contributions in progress indicates that issue/pull request is currently being worked on labels Jun 14, 2020
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Secondary, tertiary (etc) addresses, phone numbers, and emails for a patient
3 participants