care_fe icon indicating copy to clipboard operation
care_fe copied to clipboard
verified publisher icon coronasafe

Tooltip and full asset name

Open 609harsh opened this issue 1 year ago • 8 comments

Proposed Changes

  • Fixes #7787
  • Asset name is available based on space available
  • Tooltip is added so that full name is visible

@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

609harsh avatar May 15 '24 12:05 609harsh

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

A member of the Team first needs to authorize it.

vercel[bot] avatar May 15 '24 12:05 vercel[bot]

Deploy Preview for care-egov-staging ready!

Name Link
Latest commit 37d7ca82037b57101f6efd85d497b10ffc9c8c00
Latest deploy log https://app.netlify.com/sites/care-egov-staging/deploys/66a947634faa7300081e146d
Deploy Preview https://deploy-preview-7828--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.

netlify[bot] avatar May 15 '24 12:05 netlify[bot]

@609harsh

image

  • [ ] Don't show this tooltip for all asset names. Show it only for assets with lengthy names, similar to the screenshot attached in the issue. When the user hovers over those cards, display the tooltip.
  • [ ] fix the cypress failure

nihal467 avatar May 22 '24 11:05 nihal467

@609harsh

image

  • [ ] Don't show this tooltip for all asset names. Show it only for assets with lengthy names, similar to the screenshot attached in the issue. When the user hovers over those cards, display the tooltip.
  • [ ] fix the cypress failure

Hi @rithviknishad how can 1st one be done? please suggest

609harsh avatar May 24 '24 12:05 609harsh

As a simple fix, we can show tooltip only if name length is beyond certain length. Although it may not guarantee in all screen sizes and zoom levels.

Not sure if there are better alternative ways to achieve that fix via CSS. @skks1212 Any ideas?

rithviknishad avatar May 28 '24 05:05 rithviknishad

If we follow conventions, I believe we should use a simple title prop rather than a tooltip for all names regardless of length. We can also keep a toggle on the name. If you click/hover on it, rather than a tooltip, the full name is toggled. Just suggesting. Keeping the popup dependant on length doesn't seem like a good idea to me cc. @rithviknishad

shivankacker avatar May 28 '24 06:05 shivankacker

@nihal467 what should be done next? Since displaying dynamic tooltip can only be possible by length of text but it will not insure all screen sizes give similar results. i guess we can go ahead with displaying tooltip to all names or remove it for all as most of the time valid asset name will be smaller.

609harsh avatar Jun 01 '24 08:06 609harsh

@609harsh

can you try out what @skks1212 suggested

We can also keep a toggle on the name. If you click/hover on it, rather than a tooltip, the full name is toggled. Just suggesting.

and also fix the cypress issue

khavinshankar avatar Jun 05 '24 07:06 khavinshankar

@609harsh any status update on the PR

nihal467 avatar Jul 30 '24 20:07 nihal467

Closing this PR due to lack of recent progress.

bodhish avatar Sep 20 '24 15:09 bodhish