Closed ebma closed 4 weeks ago
Name | Link |
---|---|
Latest commit | b1b1b1a9694502741b0317d5c67b5c8d63cd2337 |
Latest deploy log | https://app.netlify.com/sites/rococo-souffle-a625f5/deploys/671f83ba70748d0009c31d4a |
Deploy Preview | https://deploy-preview-600--rococo-souffle-a625f5.netlify.app |
Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
@ebma @prayagd Maybe for Foucoco - Fund Wallet - should redirect to the faucet?
@ebma I changed the ExternalIcon to be a .svg file instead of react component for optimisation
Maybe for Foucoco - Fund Wallet - should redirect to the faucet?
Makes sense π objections @prayagd?
Please ignore the first point from my comment, superfast Kacper already fixed it π«‘
@Sharqiewicz are you going to add the faucet item for Foucoco or should I?
@ebma I'll do it
I refactored the code and created reusable components like <Tab>
and <CardExternalLink>
, extracting the logic to make it more generic. I also developed a custom hook, useResolvedUrl.ts
, to handle URL creation when dealing with Promise<string>
. The code is now more streamlined and split into smaller, logical, and condensed parts. Additionally, I optimized the project by converting all .svg files that donβt require customization from .tsx to .svg. No component logic should be in the /pages
directory; everything should be placed in /components
Let me know if everything is good @prayagd @ebma
@Sharqiewicz why do you prefer the basic svg files over the components? I doubt that it really makes a large difference in performance but now you had to duplicate the zenlink svg icons whereas the component-based solution would have allowed you to just define the color on the component? It's not a big deal, I'm just curious.
Besides that, let's maybe change the button color of the 'Faucet' button to something else (secondary?) and change the text to 'Top up with Faucet'. Ideally we'd also only show that button in the 'Buy' case.
Rest looks good to me and the refactorings are great overall π
@ebma We canβt set a single color for the Zenlink image since it uses both white and pink, not just white. So, I decided to go with the <img>
and .svg file. When using .svg and not inline svg allows the image to be cached and reduces the bundle size.
Great ideas, I added the changes you mentioned β
Hmm now the button looks like it's hovered/selected all the time π Sorry for the overhead with this. If we don't show it in conjunction with the 'Buy'/'Exchange' tabs (which I think is good that we don't) let's change the color back to primary.
We canβt set a single color for the Zenlink image since it uses both white and pink, not just white.
Right but I think in my version, this was already working properly as the color was only passed to the text part and not the little dot IIRC. Doesn't matter too much, I was just curious about your reasoning. Because we could consider replacing all the image components with just the image. But maybe it's better to do this on a case-by-case basis.
@prayagd can you please have another look? Your comments should be addressed now.
@ebma I think when we need the control over styling/animating the SVG then we should use inline svg. If there is no need for styling the SVG using is more efficient way - reduced bundle size ( our bundle size is big right now, we receive warnings when compiling ), enables browser caching
@ebma Thanks for changing the button color
@ebma Do we merge it? Everything is completed β
This PR adds a new page 'Fund wallet', hidden on Foucoco.
Note: The design of the tab backgrounds for Pendulum here is not accurate IMO, so I made it reflect the colors of the theme.
Closes #421 and closes #540.