Skip to content
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

O3-1710: History and Comments Section: Ordered vs Dispensed #49

Merged
merged 1 commit into from
Jan 23, 2023

Conversation

mogoodrich
Copy link
Member

@pirupius I realized after I was almost through this that there was some significiant refactoring that may affect some outstanding prescription code changes you may have, let me know if you run into issues.

A quick overview of what I did:

  • Due requirements for different left-hand colors and different tags depending on whether in the prescription view and the history and comments view I extracted the "Tile" top-level element from the Medication Event Card component and renamed that Medication Event (because I think a "card" would reference the the whole element including the Tile, but let me know if you disagree with my naming convention).

  • I also started moving things around a bit into folders by section, hopefully this makes things more organized, but it's a bit of a work in progress.

return (
<Tile className={styles.medicationEventTile}>
{actionMenu}
<div>
Copy link
Member Author

Choose a reason for hiding this comment

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

Ugh, because I renamed, looks like we aren't getting a diff from Git, but rather it looks like the file has been deleted and created. The main different in that the new "MedicationEvent" conmponent starts with this "div" here... the "Tile" and the "actionMenu" have been pulled out into the history-and-commments and prescription-details components.

{generateMedicationDispenseActionMenu(dispense)}
<Tag type="red">{t("dispense", "Dispense")}</Tag>
<MedicationEvent medicationEvent={dispense} />
</Tile>
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a clearer example of the what changed in the MedicationEventCard.... the Tile now leaves here (so we can apply custom styling) as well the action menu, and the new "Dispense" tag... the Medication Event card just displays the details of the medication event.

@@ -59,7 +79,13 @@ const PrescriptionDetails: React.FC<{
{requests &&
requests.map((request) => {
return (
<MedicationEventCard key={request.id} medicationEvent={request} />
<Tile className={styles.prescriptionTile}>
<MedicationEvent
Copy link
Member Author

Choose a reason for hiding this comment

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

Same here... the Tile has been pulled out into the Prescription Details component so that custom left-hand strip can be applied.

// TODO support completed status & will need to change expired once we change the definition of expired
// TODO will need to support potential Medication Dispense statuses or refactor into different request and dispense components
return null;
};
Copy link
Member Author

Choose a reason for hiding this comment

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

I pulled this out of a separate tag into this details component because it is now the only place where it is being used. It remains to be seen if we'll want to pull this out again.

Copy link
Member

@mseaton mseaton left a comment

Choose a reason for hiding this comment

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

Approving, but defer to @pirupius to confirm it looks ok

Copy link
Member

@pirupius pirupius left a comment

Choose a reason for hiding this comment

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

As usual love the comments 🤩 and i have no objections with the renaming and the restructure was bound to happen as the codebase grew. Please go ahead with the merge.

@mogoodrich
Copy link
Member Author

thanks @pirupius , merging!

@mogoodrich mogoodrich merged commit 8da623c into main Jan 23, 2023
@mogoodrich mogoodrich deleted the O3-1710 branch January 23, 2023 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants