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

Replace MaterialUI in FileUpload.tsx #5720

Merged
merged 9 commits into from
Jul 4, 2023

Conversation

print-Sathvik
Copy link
Contributor

@print-Sathvik print-Sathvik commented Jun 18, 2023

WHAT

🤖 Generated by Copilot at 73b140d

This pull request refactors the FileUpload component to use custom components and the Page component, and adds a custom background color option to the DialogModal component. These changes aim to improve the performance, design, and maintainability of the application.

Proposed Changes

Edit 29/06/2023:

  • Made width of buttons same in mobile view
  • Fixed the buttons overflowing from its parent component when page width is less(<1280px)
    .
    @coronasafe/care-fe-code-reviewers @coronasafe/code-reviewers

HOW

🤖 Generated by Copilot at 73b140d

  • Refactor FileUpload component to use custom components instead of Material UI components and legacy components, and to match the design specifications (link, link, link, link, link, link)
  • Replace PageTitle component with Page component in FileUpload component and its subcomponents, to use a more consistent and flexible layout for the pages (link, link, link)
  • Add conditional class name to DialogModal component, to allow customizing the background color by passing a bg- class name as a prop (link)

@print-Sathvik print-Sathvik requested a review from a team June 18, 2023 21:34
@print-Sathvik print-Sathvik requested a review from a team as a code owner June 18, 2023 21:34
@vercel
Copy link

vercel bot commented Jun 18, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
care-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 3, 2023 4:57pm

@netlify
Copy link

netlify bot commented Jun 18, 2023

Deploy Preview for care-egov-staging ready!

Name Link
🔨 Latest commit 16159e7
🔍 Latest deploy log https://app.netlify.com/sites/care-egov-staging/deploys/64a2fdc4cba5530008e50f11
😎 Deploy Preview https://deploy-preview-5720--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.

src/Components/Common/Dialog.tsx Outdated Show resolved Hide resolved
src/Components/Patient/FileUpload.tsx Outdated Show resolved Hide resolved
@nihal467
Copy link
Member

@rithviknishad @print-Sathvik
image

  • make all the button in full width in mobile view

@print-Sathvik
Copy link
Contributor Author

@rithviknishad @print-Sathvik image

  • make all the button in full width in mobile view

That was taken up by many others in the issue #5540 So I left that on purpose. Should I assign that issue to myself and continue with that @nihal467 ?

@rithviknishad
Copy link
Member

@print-Sathvik you can do it along with this PR and link that issue too.

@print-Sathvik
Copy link
Contributor Author

Ok @rithviknishad , that's done. I noticed one more issue. Those buttons are overflowing when width is decreased. I will fix that and create a commit evening after going home

@rithviknishad
Copy link
Member

@nihal467 that space is for the error for that field. In existing staging when error comes, layout shift happens. But layout shift is not good, hence new components automatically keeps the spacing consistent to prevent layout shifts.

@rithviknishad
Copy link
Member

@print-Sathvik maybe we can add some spacing above the field so that it looks good in all views

@nihal467
Copy link
Member

nihal467 commented Jun 30, 2023

@rithviknishad @print-Sathvik

image

the above image is a mockup I made, can we have something similar?

@print-Sathvik
Copy link
Contributor Author

print-Sathvik commented Jun 30, 2023

Ok @nihal467 , will do that tonight

@nihal467
Copy link
Member

nihal467 commented Jul 3, 2023

LGTM

@khavinshankar khavinshankar merged commit 6ed8841 into coronasafe:develop Jul 4, 2023
26 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants