#6417 tooltip update gallery #6448
Conversation
prachi00
requested review from
roiLeo and
vikiival
and removed request for
a team
✅ Deploy Preview for koda-canary ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
AI-Generated Summary: This pull request includes changes associated with the update of the tooltip used in the gallery. Modifications are made to the The |
|
@exezbcz are we okay with tooltip showing up in left side? because with top, it overflows out of the screen |
✅ Deploy Preview for koda-canary ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
components/gallery/GalleryItemAction/GalleryItemActionType/GalleryItemBuy.vue
Outdated
Show resolved
Hide resolved
styles/components/_gallery-item.scss
Outdated
|
|
||
| .neo-tooltip, | ||
| .is-neo { | ||
| height: 54px; |
There was a problem hiding this comment.
is it necessary to have a fixed height?
There was a problem hiding this comment.
yeah else the button will be of a different height
best would be showing "Kusama" for RMRK1 and RMRK2 |
prury
added
the
S-changes-requested-🤞
…leryItemBuy.vue Co-authored-by: roiLeo <[email protected]>
|
@prury whats ramp page? can you give the url where it should redirect? |
fiat on ramp is a mechanism that allows you to buy crypto directly with your credit card
|
|
we need to figure out how this is suppose to look on mobile as the text is too big @prury |
@prury is this okay?
|
yes, it works, on the right side of the button aswell if it fits |
added top to all now, it will look like this, web and mobile
|
Wonderful, only thing left now is this ^ |
|
done |
RMRK2 still shows RMRK chain, should be Kusama, otherwise it will induce user to error making it think that he needs assets directly on the RMRK chain, which isn't true, the rest of the other chains are showing the correct information. |
| {{ | ||
| $t('tooltip.notEnoughBalanceChain', { | ||
| chain: | ||
| chainInfo[urlPrefix][0].toUpperCase() + |
There was a problem hiding this comment.
better to create chainNames on this file https://github.com/kodadot/nft-gallery/blob/main/libs/static/src/chains.ts
using switch case or object to return chain name based on prefix. for example
case 'rmrk'
case 'ksm'
return 'kusama'
There was a problem hiding this comment.
We already have chain names there 👀
There was a problem hiding this comment.
yeah why are we adding it twice? can we change ksm to 'kusama' instead of rmrk? @preschian @roiLeo @vikiival
There was a problem hiding this comment.
on the referenced issue, the tool tip is showing the chain name and not its token/abbreviation like it stands now:
not that one, chainInfo is from the developer's perspective. this is from the user's perspective #6448 (comment). if the user clicks "Add Funds", it means they need some token on "kusama" chain, not on "rmrk" chain
components/gallery/GalleryItemAction/GalleryItemActionType/GalleryItemBuy.vue
Outdated
Show resolved
Hide resolved
Co-authored-by: Preschian Febryantara <[email protected]>
|
Code Climate has analyzed commit 2f490b6 and detected 0 issues on this pull request. View more on Code Climate. |
|
chains are perfect now! thanks to everyone involved but oops, seems like mobile is wrong again:
|
button needs full-width to show the entire tooltip on mobile. ref: #6448 (comment) |
|
pay 20 usd continue in #6512 |
|
😍 Perfect, I’ve sent the payout 🪅 Let’s grab another issue and get rewarded! |
Thank you for your contribution to the KodaDot - One Stop Shop for Polkadot NFTs.
👇 __ Let's make a quick check before the contribution.
PR Type
Needs QA check
Context
Did your issue had any of the "$" label on it?
Screenshot 📸
Copilot Summary