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

Rearranged facility home layout #7315

Merged
merged 13 commits into from
Apr 24, 2024

Conversation

kshitijv256
Copy link
Contributor

@kshitijv256 kshitijv256 commented Feb 29, 2024

Proposed Changes

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

Screenshots

Mobile

Screenshot from 2024-03-20 22-12-38

Tablet

Screenshot from 2024-03-20 22-15-50

Laptop 14 inch

Screenshot from 2024-03-20 22-11-43

24 inch 2K screen

Screenshot from 2024-03-20 22-12-02

@kshitijv256 kshitijv256 requested a review from a team as a code owner February 29, 2024 13:23
Copy link

vercel bot commented Feb 29, 2024

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 Mar 4, 2024 6:16am

Copy link

netlify bot commented Feb 29, 2024

Deploy Preview for care-egov-staging ready!

Name Link
🔨 Latest commit 21345d4
🔍 Latest deploy log https://app.netlify.com/sites/care-egov-staging/deploys/6628b1dd3439e9000804200c
😎 Deploy Preview https://deploy-preview-7315--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.

@nihal467
Copy link
Member

nihal467 commented Mar 5, 2024

image

@aparnacoronasafe feel like there are a lot of white spacing issues occurring, can you look into the UI once again

@nihal467 nihal467 added the question Further information is requested label Mar 5, 2024
@aparnacoronasafe
Copy link
Member

image

@aparnacoronasafe feel like there are a lot of white spacing issues occurring, can you look into the UI once again

@nihal467 whats your screen size? I am on a 13inch mac.

This is what I am seeing at 100% screen size
Uploading Screenshot 2024-03-06 at 11.30.18 AM.png…

I think it is good to go

@nihal467
Copy link
Member

nihal467 commented Mar 6, 2024

@aparnacoronasafe I am using 15.6-inch laptop screen as well as 27-inch monitor screen , on both of them, they are having a huge white spacing issue

@aparnacoronasafe
Copy link
Member

@nihal467 lets release this and think of iterative improvements in design?

@rithviknishad
Copy link
Member

rithviknishad commented Mar 11, 2024

IMO the current staging itself seems to be a better layout than the one in this PR for both resolutions.

@nihal467
Copy link
Member

@gigincg can you review this

@gigincg
Copy link
Member

gigincg commented Mar 20, 2024

@kshitijv256 Can you update the PR based on the Image Reference in the Issue. Padding for the Picture, and the Alignment of the Buttons. Also, post Screenshots for Mobile, Tab, HD & Full HD Screens. Let's wrap this ASAP.

Copy link

vercel bot commented Mar 20, 2024

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

A member of the Team first needs to authorize it.

@kshitijv256
Copy link
Contributor Author

@gigincg @nihal467 @rithviknishad updated screenshots for different screen sizes

@rithviknishad
Copy link
Member

rithviknishad commented Mar 21, 2024

There needs to be a a justify-between between the manage facility and rest of the buttons in desktop view as per the issue ref. image

image

@rithviknishad rithviknishad removed question Further information is requested needs testing discussion required labels Mar 21, 2024
@rithviknishad
Copy link
Member

Please refer the ref. screenshot in the issue

image

The vertical height of the div containing the buttons is limited to the same height as the facility picture.

@kshitijv256
Copy link
Contributor Author

Changed as requested @rithviknishad

Screenshot from 2024-03-22 19-00-36

@rithviknishad
Copy link
Member

rithviknishad commented Mar 23, 2024

Layout and alignments looks good to me, but the Edit image button on hover no longer seems to be present

Develop deployment on hover:
image

This branch on hover:
image

@rithviknishad
Copy link
Member

Can we also give the image a rounded-lg border?

@kshitijv256
Copy link
Contributor Author

@rithviknishad tooltip shows when you hover on top part of image

Screenshot from 2024-03-23 12-46-50

Copy link

Hi, This pr has been automatically marked as stale because it has not had any recent activity. It will be automatically closed if no further activity occurs for 7 more days. Thank you for your contributions.

@github-actions github-actions bot added stale merge conflict pull requests with merge conflict labels Mar 31, 2024
Copy link

github-actions bot commented Apr 2, 2024

👋 Hi, @kshitijv256,
Conflicts have been detected against the base branch. Please rebase your branch against the base branch.


This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there.

@github-actions github-actions bot removed merge conflict pull requests with merge conflict stale labels Apr 2, 2024
@@ -106,7 +106,7 @@ export const FacilityHome = (props: any) => {
const editCoverImageTooltip = hasPermissionToEditCoverImage && (
<div
id="facility-coverimage"
className="absolute right-0 top-0 z-10 flex h-full w-full flex-col items-center justify-center bg-black text-sm text-gray-300 opacity-0 transition-opacity hover:opacity-60 md:h-[88px]"
className="absolute right-0 top-0 z-10 flex h-full w-full flex-col items-center justify-center rounded-t-lg bg-black text-sm text-gray-300 opacity-0 transition-[opacity] hover:opacity-60 md:h-[88px]"
Copy link
Member

Choose a reason for hiding this comment

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

@kshitijv256 please double check transistion-[opacity] class

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@khavinshankar
I missed it as I was trying out other transitions there, changed it back to transition-opacity

@khavinshankar khavinshankar merged commit b4b70e4 into coronasafe:develop Apr 24, 2024
12 of 16 checks passed
Copy link

@kshitijv256 Your efforts have helped advance digital healthcare and TeleICU systems. 🚀 Thank you for taking the time out to make CARE better. We hope you continue to innovate and contribute; your impact is immense! 🙌

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.

Improving UI of Facility dashboard
7 participants