-
Notifications
You must be signed in to change notification settings - Fork 65
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
feat: Style improvements for buttons and tooltips #147
feat: Style improvements for buttons and tooltips #147
Conversation
…udes a noticeable hover effect.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@NikolaiKryshnev is attempting to deploy a commit to the Abacus Works Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for these improvements! Just a few small things to fix please
src/styles/custom.css
Outdated
/* Fix for tooltips breaking layout on mobile devices (<526px width). */ | ||
|
||
|
||
@media (max-width: 526px) {#gas-info, #transaction-info, #message-info{max-width: calc(100% - 10px);}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use tailwind classes instead of creating a new css file.
If you need to do specific amounts like 526px, you can use a class like max-w-[526px]
, but it would be better if you can make this work with just the max-w
classes in tailwind's base set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion! Unfortunately, this appears to be a limitation or bug in the Tooltip library. When we try to pass a className like max-w-[calc(100%-10px)] sm:max-w-[526px], it gets applied to an unexpected tag instead of the actual tooltip. This behavior prevents us from fully leveraging Tailwind classes in this case. For now, me had to rely on a custom CSS file to address this issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a property inside the Tooltip called data-tooltip-class-name
which directly modifies the className
for the tooltip component. I vote we also move this part to the widgets lib @jmrossy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@NikolaiKryshnev would you like to give this classname-based approach a try? Also from the CI failure it looks like you need to run yarn prettier
to fix the formatting issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jmrossy Thanks for the suggestion! I gave the data-tooltip-class-name approach a try, and it seems to work well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like prettier is still failing due to the extra space is SearchBar but I'll merge as is and fix it for you.
Fix: Ensure proper tooltip rendering on mobile devices
BEFORE:
before.mp4
AFTER
after.mp4
Enhance button visibility with a hover effect (pink to blue transition).
BEFORE:
before_2.mp4
AFTER:
after_2.mp4
Fixed: Button now covers parent area for intuitive usability and incl…
BEFORE:
before3.mp4
AFTER:
after3.mp4