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

Move export asset button beside asset name and improve import/export icons and fix button border control #4244

Merged
merged 12 commits into from
Dec 15, 2022

Conversation

rithviknishad
Copy link
Member

@rithviknishad rithviknishad commented Dec 7, 2022

Proposed Changes

Move asset export beside the asset name and added a tooltip.

image

Improved the import and export icon in asset list page

image

ButtonV2 can now be ghost without border

image

@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

@rithviknishad rithviknishad requested a review from a team December 7, 2022 11:07
@rithviknishad rithviknishad requested a review from a team as a code owner December 7, 2022 11:07
@netlify
Copy link

netlify bot commented Dec 7, 2022

Deploy Preview for care-egov-staging ready!

Name Link
🔨 Latest commit b228dcc
🔍 Latest deploy log https://app.netlify.com/sites/care-egov-staging/deploys/6398299a32320a0008de5427
😎 Deploy Preview https://deploy-preview-4244--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 settings.

@vercel
Copy link

vercel bot commented Dec 7, 2022

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

Name Status Preview Updated
care-storybook ✅ Ready (Inspect) Visit Preview Dec 13, 2022 at 7:29AM (UTC)

@nihal467
Copy link
Member

nihal467 commented Dec 7, 2022

LGTM, @rithviknishad marking it team discussion required, to get a final opinion from the team

@nihal467
Copy link
Member

nihal467 commented Dec 7, 2022

@rithviknishad will you be working on the changes, from removing the button and adding a download icon in the same row as the asset name

@rithviknishad
Copy link
Member Author

rithviknishad commented Dec 8, 2022

@nihal467 the following changes were made (screenshots updated in PR's first comment):

  • Removed the original asset export button beside configure asset.
  • Added an icon button with a tooltip 'Export as JSON' beside the asset name
  • Updated the import and export icons on the Asset List page.
  • ButtonV2 got a new controllable border prop for finer design control. (Visible in advanced patient filters, although not directly related to the objective of this PR, to achieve the no-border style for the export button, this change had to be present in this PR)

Ready for testing.

@rithviknishad rithviknishad changed the title Made 'Export Asset' as secondary button Move export asset button beside asset name and improve import/export icons Dec 8, 2022
@rithviknishad rithviknishad changed the title Move export asset button beside asset name and improve import/export icons Move export asset button beside asset name and improve import/export icons and fix button border control Dec 8, 2022
@github-actions
Copy link

github-actions bot commented Dec 8, 2022

👋 Hi, @rithviknishad,
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 added the merge conflict pull requests with merge conflict label Dec 8, 2022
@github-actions
Copy link

github-actions bot commented Dec 8, 2022

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

@nihal467
Copy link
Member

nihal467 commented Dec 8, 2022

LGTM but @rithviknishad work on the merge conflict

@rithviknishad rithviknishad removed the merge conflict pull requests with merge conflict label Dec 8, 2022
@github-actions github-actions bot added the Deploy-Failed Deplyment is not showing preview label Dec 8, 2022
@netlify
Copy link

netlify bot commented Dec 8, 2022

Deploy Preview for care-net failed.

Name Link
🔨 Latest commit 516ddee
🔍 Latest deploy log https://app.netlify.com/sites/care-net/deploys/639230d7ce84210008e31050

@rithviknishad
Copy link
Member Author

@nihal467 i've fixed the merge conflict

@nihal467
Copy link
Member

nihal467 commented Dec 13, 2022

@rithviknishad
image

the page is getting extended to the right side, due to the tooltip, in responsive view

@sonarcloud
Copy link

sonarcloud bot commented Dec 13, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@nihal467
Copy link
Member

LGTM

@khavinshankar khavinshankar merged commit e810fd4 into coronasafe:develop Dec 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants