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

zoom in out functionality added and added responsiveness for mobile s… #8336

Closed
wants to merge 1 commit into from

Conversation

ayushpatil2122
Copy link
Contributor

@ayushpatil2122 ayushpatil2122 commented Aug 16, 2024

…creen

Proposed Changes

Added a zoom in and zoom out in PrintPreview component added a button for zoom in and zoom out make a PrintPreview component responsive for mobile screen

@coronasafe/care-fe-code-reviewers @coronasafe/code-reviewers

Merge Checklist

  • Add specs that demonstrate bug / test a new feature.
  • Update product documentation.
  • Ensure that UI text is kept in I18n files.
  • Prep screenshot or demo video for changelog entry, and attach it to issue.
  • Request for Peer Reviews
  • Completion of QA

@ayushpatil2122 ayushpatil2122 requested a review from a team as a code owner August 16, 2024 18:56
Copy link

vercel bot commented Aug 16, 2024

@Coderayush13 is attempting to deploy a commit to the Open Healthcare Network Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

netlify bot commented Aug 16, 2024

Deploy Preview for care-egov-staging ready!

Name Link
🔨 Latest commit 2d0bec3
🔍 Latest deploy log https://app.netlify.com/sites/care-egov-staging/deploys/66bfa0fd14baa60008df11b4
😎 Deploy Preview https://deploy-preview-8336--care-egov-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@rithviknishad rithviknishad left a comment

Choose a reason for hiding this comment

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

Could you discard all changes made to the Prescription's Print Preview? Adding zoom support alone to the PrintPreview component should be fine right?

Comment on lines +52 to +84
<div className="mb-6 grid grid-cols-1 md:grid-cols-2 lg:grid-cols-4 gap-y-1.5 border-2 border-secondary-400 p-2">
<PatientDetail name="Patient" className="col-span-1 md:col-span-2 lg:col-span-2">
{patient && (
<>
<span className="uppercase">{patient.name}</span> -{" "}
{t(`GENDER__${patient.gender}`)},{" "}
{patientAgeInYears(patient).toString()}yrs
</>
)}
</PatientDetail>
<PatientDetail name="IP/OP No." className="col-span-1 md:col-span-1 lg:col-span-1">
{encounter?.patient_no}
</PatientDetail>

<PatientDetail
name={
encounter
? `${t(`encounter_suggestion__${encounter.suggestion}`)} on`
: ""
}
className="col-span-5"
>
{formatDate(encounter?.encounter_date)}
</PatientDetail>
<PatientDetail name="Bed" className="col-span-3">
{encounter?.current_bed?.bed_object.location_object?.name}
{" - "}
{encounter?.current_bed?.bed_object.name}
</PatientDetail>
<PatientDetail
name={
encounter
? `${t(`encounter_suggestion__${encounter.suggestion}`)} on`
: ""
}
className="col-span-1 md:col-span-2 lg:col-span-2"
>
{formatDate(encounter?.encounter_date)}
</PatientDetail>
<PatientDetail name="Bed" className="col-span-1 md:col-span-1 lg:col-span-1">
{encounter?.current_bed?.bed_object.location_object?.name}
{" - "}
{encounter?.current_bed?.bed_object.name}
</PatientDetail>

<PatientDetail name="Allergy to medication" className="col-span-1 md:col-span-2 lg:col-span-4">
{patient?.allergies ?? "None"}
</PatientDetail>
Copy link
Member

Choose a reason for hiding this comment

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

@ayushpatil2122 Like already mentioned in the issue, let's not make the print preview child responsive based on screen size. This would mean, the print output could differ on different screen size than what was shown in the preview.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry i will fix that

<div className="pt-12">
<p className="font-medium text-secondary-800">
<div className="pt-12 px-4">
<p className="font-medium text-secondary-800 text-base">
Copy link
Member

Choose a reason for hiding this comment

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

Let's not make changes to things that were not asked in the issue.

Suggested change
<p className="font-medium text-secondary-800 text-base">
<p className="font-medium text-secondary-800">

@@ -150,31 +153,34 @@ const PrescriptionsTable = ({
}

if (!items.length) {
return;
return <div>No prescriptions available</div>; // Add a fallback message for empty items
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return <div>No prescriptions available</div>; // Add a fallback message for empty items
return <div>No prescriptions available</div>;

))}
</tbody>
</table>
<div className="overflow-x-auto">
Copy link
Member

Choose a reason for hiding this comment

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

Why was this made scrollable? Print preview content should not be scrollable within the paper view.

@rithviknishad
Copy link
Member

@ayushpatil2122 Were you able to make the changes?

@github-actions github-actions bot removed the stale label Aug 30, 2024
@rithviknishad
Copy link
Member

Closing due to inactivity and completed in #8487

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for zooming in and out for PrintPreview component
3 participants