search

scaffold-eth / se-2-challenges

SpeedRunEthereum challenges (Powered by Scaffold-ETH 2)
https://speedrunethereum.com
MIT License
72 stars 138 forks source link

Challenge 0 wagmi 2 backmerge #171

Closed rin-st closed 3 months ago

rin-st commented 4 months ago

When clicking on "Transfers" tab we get this error: Error: Contract not found, but the information loads correctly.

You mean error in console, right? Added a quick fix for now. But looks like it's an at least 5-month old error, will add pr to se-2 soon

When you paste an address on the "Transfer To" field for the first time after loading the "My NFTs" tab, we get a small flickering. Video here

It's because of skeleton loading - input becomes wider, and hence card too. You can try to use ens names inside input, you'll see that card becomes wide. It worked that way already, I don't see the good way to fix it. We can fix width of the image so it will not widen on ens loading. So it will look like

image

instead of

image
rin-st commented 4 months ago

created an issue for image width fix, I think it's better to fix it a bit later https://github.com/scaffold-eth/se-2-challenges/issues/172

rin-st commented 4 months ago

useScaffoldEventHistory update pr (could be merged later)

Pabl0cks commented 4 months ago

You mean error in console, right? Added a quick fix for now. But looks like it's an at least 5-month old error, will add pr to se-2 soon

Yeah! Just checked the SE-2 PR 🔥♥. Since we are still with Ch. 0, should we wait to merge it, and then backmerge it to base & chal0 so is fixed on all challenges in this PR iteration?

It's because of skeleton loading - input becomes wider, and hence card too.

created an issue for image width fix, I think it's better to fix it a bit later https://github.com/scaffold-eth/se-2-challenges/issues/172

Ok! It was a very nitpicky thing, only commented it because it seemed to me that in an older version of Chal. 0 it was not happening, if not I wouldn't have raised it here probably 🙌

rin-st commented 4 months ago

Yeah! Just checked the SE-2 PR 🔥♥. Since we are still with Ch. 0, should we wait to merge it, and then backmerge it to base & chal0 so is fixed on all challenges in this PR iteration?

Yes, let's wait then

Ok! It was a very nitpicky thing, only commented it because it seemed to me that in an older version of Chal. 0 it was not happening, if not I wouldn't have raised it here probably 🙌

It exists since start :)

rin-st commented 4 months ago

Yeah! Just checked the SE-2 PR 🔥♥. Since we are still with Ch. 0, should we wait to merge it, and then backmerge it to base & chal0 so is fixed on all challenges in this PR iteration?

Added events hook change and https://github.com/scaffold-eth/scaffold-eth-2/pull/825 . Working fine for me, hope for you too 😄

I think I'll merge it again together with challenges 1 and 2

Pabl0cks commented 4 months ago

Added events hook change and https://github.com/scaffold-eth/scaffold-eth-2/pull/825 . Working fine for me, hope for you too 😄

Working well to me!

I think I'll merge it again together with challenges 1 and 2

💯

rin-st commented 4 months ago

Added last changes, tested. Working fine.

Pabl0cks commented 4 months ago

I'm getting a strange render problem with the copy address button with the "View QR Code" option from RainbowKit.

If I view QR from a Metamask wallet, it's fine, but if I'm connected to burner wallet, copy icon renders super small:

image

Pabl0cks commented 4 months ago

The challenge is still working fine to me. Should we change the spacing in AddressQRCodeModal.tsx from space-x-4 to 1 or 0 to fix that kind of edge cases?

If we decide to tweak something, I'd probably just do it in se-2 and backmerge it in the next big backmerge to challenges, since is a really minor bug.

rin-st commented 4 months ago

I'd probably just do it in se-2 and backmerge it in the next big backmerge to challenges, since is a really minor bug.

You can create pr to se-2 and we'll add it to the current backmerge. Looks like we need to fix other bugs related to wagmi 2 in the current backmerge anyway

Pabl0cks commented 4 months ago

You can create pr to se-2 and we'll add it to the current backmerge. Looks like we need to fix other bugs related to wagmi 2 in the current backmerge anyway

Tried reproducing it in SE-2, it's not happening for the same address. The problem is a combination of Challenges font and some longer weight addresses due to their characters size.

Should we change spacing a bit in SE-2 to fix that edge case (and other posible builds with SE-2 and big weight fonts), or should just leave it like that.

rin-st commented 4 months ago

I think it's better to fix it in se-2

Pabl0cks commented 4 months ago

Did last tests, looking good! 🙌

Pabl0cks commented 3 months ago

While doing a fast check with the last burner commits on the challenges, realized of a SE-2 issue with the burner, a bit edge case, but maybe is good to add the fix when we have it.

rin-st commented 3 months ago

Merging this since the burner issue is closed. Thank you a lot @Pabl0cks ! ❤️