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

Results table modal #79

Merged
merged 9 commits into from
Oct 14, 2022
Merged

Results table modal #79

merged 9 commits into from
Oct 14, 2022

Conversation

jhughes982
Copy link
Collaborator

@jhughes982 jhughes982 added the enhancement New feature or request label Oct 14, 2022
@jhughes982 jhughes982 requested a review from stefpiatek October 14, 2022 12:29
@jhughes982 jhughes982 self-assigned this Oct 14, 2022
@jhughes982 jhughes982 temporarily deployed to test-env October 14, 2022 12:29 Inactive
@jhughes982 jhughes982 temporarily deployed to test-env October 14, 2022 13:19 Inactive
Copy link
Collaborator

@stefpiatek stefpiatek left a comment

Choose a reason for hiding this comment

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

🎉 thanks for doing this - not sure if my comment on guard clauses is that astounding but does save a small bit I hope

Comment on lines 85 to 92
if (
!patient.id ||
!patient.identifier ||
!patient.identifier[0].value ||
!patient.name ||
!patient.name[0].given ||
!patient.name[0].family
) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We could make this a bit more clear by ? and also using !(with && and && linking). so maybe something like this?

Suggested change
if (
!patient.id ||
!patient.identifier ||
!patient.identifier[0].value ||
!patient.name ||
!patient.name[0].given ||
!patient.name[0].family
) {
if (!(
patient.id &&
patient.identifier?.get(0)?.value &&
patient.name?.get(0)?.given &&
patient.name?.get(0)?.family
)) {

if (component?.code?.coding?.[0].code === HGVS_CDNA_LOINC) {
return component?.valueCodeableConcept?.coding?.[0].display;
if (
!component ||
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this line is necessary in the guard as it's covered below

@jhughes982 jhughes982 temporarily deployed to test-env October 14, 2022 15:11 Inactive
@jhughes982
Copy link
Collaborator Author

Thanks for the suggestions. I think that is now a bit leaner and more semantically correct. Needs a bit of concentration to follow through the logic though unfortunately.

@jhughes982 jhughes982 merged commit e1ba6b2 into main Oct 14, 2022
@jhughes982 jhughes982 deleted the results-table-modal branch October 14, 2022 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
2 participants