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

chores: Fixed responsiveness of the expanded sort dropdown in the patients page #7321

Closed
wants to merge 17 commits into from

Conversation

ab1123
Copy link

@ab1123 ab1123 commented Mar 1, 2024

Proposed Changes

@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

Screenshot

PR image 2
PR image 1

@ab1123 ab1123 requested a review from a team as a code owner March 1, 2024 22:54
Copy link

vercel bot commented Mar 1, 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 11, 2024 9:00pm

Copy link

netlify bot commented Mar 1, 2024

Deploy Preview for care-egov-staging ready!

Name Link
🔨 Latest commit 60e6a15
🔍 Latest deploy log https://app.netlify.com/sites/care-egov-staging/deploys/661e676d3f87620008c41555
😎 Deploy Preview https://deploy-preview-7321--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/components/Menu.tsx Outdated Show resolved Hide resolved
@nihal467
Copy link
Member

nihal467 commented Mar 5, 2024

@ab1123 can we make it full-width / grow to use available space in smaller than desktop sizes?

image

@ab1123
Copy link
Author

ab1123 commented Mar 6, 2024

@ab1123 can we make it full-width / grow to use available space in smaller than desktop sizes?

image

Yes sure sir, I will edit the necessary changes

@ab1123
Copy link
Author

ab1123 commented Mar 11, 2024

I have pushed the necessary details and henceforth I request for maintainer's review

@rithviknishad
Copy link
Member

rithviknishad commented Mar 12, 2024

It is still not full-width on mobile screens

image

This is what is expected:

image

@@ -102,7 +102,7 @@ const ButtonV2 = ({
}: ButtonProps) => {
const className = classNames(
props.className,
"inline-flex h-min cursor-pointer items-center justify-center gap-2 whitespace-pre font-medium outline-offset-1 transition-all duration-200 ease-in-out disabled:cursor-not-allowed disabled:bg-gray-200 disabled:text-gray-500",
"inline-flex h-max cursor-pointer items-center justify-center gap-2 whitespace-pre font-medium outline-offset-1 transition-all duration-200 ease-in-out disabled:cursor-not-allowed disabled:bg-gray-200 disabled:text-gray-500",
Copy link
Member

Choose a reason for hiding this comment

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

avoid changing classes of the reusable button component as this would affect the entire platform.

Copy link
Author

Choose a reason for hiding this comment

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

Sure sir. I will implement that

/>
)}
<div className="tooltip ml-4">
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
<div className="tooltip ml-4">
<div className="tooltip ml-4 flex-1">

parse: preventDuplicatePatientsDuetoPolicyId,
},
]}
<div className="vs:block sm:flex md:flex">
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
<div className="vs:block sm:flex md:flex">
<div className="vs:block sm:flex md:flex w-full">

@github-actions github-actions bot added the merge conflict pull requests with merge conflict label Mar 22, 2024
Copy link

👋 Hi, @ab1123,
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.

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 the stale label Mar 30, 2024
@github-actions github-actions bot removed the stale label Mar 31, 2024
@ab1123
Copy link
Author

ab1123 commented Mar 31, 2024

@rithviknishad can you please review the latest commit as all the requested change are migrated in the latest commit.

@rithviknishad rithviknishad added needs testing and removed test failed changes required merge conflict pull requests with merge conflict labels Apr 1, 2024
@nihal467
Copy link
Member

nihal467 commented Apr 2, 2024

image

@ab1123 the switching tab UI is breaking in normal desktop view

@ab1123
Copy link
Author

ab1123 commented Apr 5, 2024

Sure sir, I will make the necessary changes and push a new commit

@ab1123
Copy link
Author

ab1123 commented Apr 15, 2024

@nihal467 kindly verify the changes made

@nihal467
Copy link
Member

nihal467 commented Apr 16, 2024

@ab1123
image
image

  • move the button to the right side, as it is currently in the staging
  • don't change the export icon
  • make the same button behavior as the add patient button in responsive

@rithviknishad
Copy link
Member

Closing due to inactivity. Feel free to re-open the PR.

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.

VIsual Bug: Issue in responsiveness
4 participants