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

Bug report: poor tooltip position - find wallet #12643

Closed
1 of 2 tasks
konopkja opened this issue Apr 3, 2024 · 12 comments · Fixed by #12779
Closed
1 of 2 tasks

Bug report: poor tooltip position - find wallet #12643

konopkja opened this issue Apr 3, 2024 · 12 comments · Fixed by #12779
Assignees
Labels
bug 🐛 Something isn't working dev required This requires developer resources good first issue Good item to try if you're new to contributing

Comments

@konopkja
Copy link
Contributor

konopkja commented Apr 3, 2024

Describe the bug

in some instances the tooltip is too far from the text
Screenshot 2024-04-03 at 11 55 09

To reproduce

  1. Go to https://ethereum.org/en/wallets/find-wallet/
  2. Click on wallet detail

Expected behavior

the tooltip should be right after the text without blank gap

Screenshots

No response

Desktop (please complete the following information)

No response

Smartphone (please complete the following information)

No response

Additional context

No response

Would you like to work on this issue?

  • Yes
  • No
@konopkja konopkja added bug 🐛 Something isn't working dev required This requires developer resources labels Apr 3, 2024
@github-actions github-actions bot added the needs triage 📥 This issue needs triaged before being worked on label Apr 3, 2024
@Dharmik79
Copy link

@konopkja I would like to work on this issue

@konopkja
Copy link
Contributor Author

konopkja commented Apr 3, 2024

@Dharmik79 thank you so much for offering your help! I will assign you the task :) assigned you the issue. @pabloped since we had some issues handling this internally adding you here for review.

@wackerow
Copy link
Member

wackerow commented Apr 8, 2024

I can't find my original comment at the moment, but yeah just noting this is a little tricky because when the text wraps from being too long, the browser doesn't collapse the container width to the now-wrapped text, it just fills the full width (which is what pushes that (i) all the way to the right edge when the text wraps)...

We could make this display: inline; with the text, but then the (i) would be like this:

Some really long example
name here (i)

...which is not what @konopkja is requesting here. Not sure the best way to keep the (i) right next to the text block when it's multiple lines tbh.

@konopkja
Copy link
Contributor Author

Hello @Dharmik79 what is the status on this task?

@Dharmik79
Copy link

@konopkja The issue is with the css property, and using display:inline it does not fix the issue. We need to use another method for that.

@konopkja
Copy link
Contributor Author

cc @pettinarip @wackerow

@pettinarip
Copy link
Member

Hmm I don't have atm a better solution than the one @wackerow mentioned. I think moving the (i) icon inside the paragraph would make it follow the width of the text. Is not exactly what was requested but I think it will look good and consistent as well.

@Dharmik79
Copy link

Dharmik79 commented Apr 17, 2024

@pettinarip @wackerow Can we remove the (i) icon and set the tooltip from the text itself?

@konopkja
Copy link
Contributor Author

@pettinarip @wackerow Can we remove the (i) icon and set the tooltip from the text itself?

its good idea to consider different solutions, this one though would probably not be good for mobile devices apart from also being less clear. Id advise against it.

@wackerow
Copy link
Member

One approach that I mentioned elsewhere is slightly messier, but it seems to work:

Inside src/components/FindWallet/WalletTable/WalletMoreInfoCategory.tsx, before if (feature.category === sectionName) on line 61, insert:

const lastWord = feature.label.split(" ").pop()

Then rearrange the tooltip below as follows:

                <Text px={1} lineHeight={1}>
                  {feature.label}

                  <Text as="span" whiteSpace="nowrap">
                    {lastWord}
                    <Tooltip
                      content={
                        <Text color="body.base !important">
                          {t(walletFilterData[feature.filterKey].description)}
                        </Text>
                      }
                    >
                      <Box as="span" ms="2">
                        <Icon as={MdInfoOutline} color={featureColor} />
                      </Box>
                    </Tooltip>
                  </Text>
                </Text>
  1. Pulls the Tooltip into the feature.label span, so it lays inline with the text
  2. Adds a margin-start of half a rem (ms="2") to icon box
  3. Breaks off the last word and wraps it in a nowrap span with the icon, so the icon never lays on it's own line alone

Probably a cleaner way to do it, but this is the result:
ltr:
image

rtl:
image

May need to just clean up the vertical alignment a little with the above, but it seems to be a solution to the posted problem.m

@Dharmik79
Copy link

@wackerow This solution looks pretty good, I will implement this and will create the pull request for the same.

Copy link
Contributor

This issue is stale because it has been open 30 days with no activity.

@github-actions github-actions bot added the Status: Stale This issue is stale because it has been open 30 days with no activity. label May 19, 2024
@wackerow wackerow added good first issue Good item to try if you're new to contributing and removed Status: Stale This issue is stale because it has been open 30 days with no activity. needs triage 📥 This issue needs triaged before being worked on good first issue Good item to try if you're new to contributing labels Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something isn't working dev required This requires developer resources good first issue Good item to try if you're new to contributing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants