Bug fixes#113
Conversation
✅ Deploy Preview for test-cheddar-ecosystem ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Mart-Dan-Rossi
left a comment
There was a problem hiding this comment.
In addition to comments made, i found that, even when some texts and styles were changed pointing that after a match the cheddar earned should be not minted inmidiatly and claimed all toghether after clicking a button, this is not happening.
RodrigoCSolari
left a comment
There was a problem hiding this comment.
I agree with the changes that Martin asked for.
|
@Mart-Dan-Rossi @RodrigoCSolari I have resolved the comments, and removed the board related changes (randomness and unreachable cell), so we can merge the other fixed bugs. |
| : 'playerBackgroundElementOnTop'; | ||
|
|
||
| // Combine multiple CSS classes using the 'clsx' utility function to handle conditional styling | ||
| return clsx( |
There was a problem hiding this comment.
We have multiple lines in the project where we should use this function to be consistent on the way to do this stuff. I know it's not a priority right now to refactor the code but it doesn't look good idea to comment every place where we use this function. I was expecting some better documentation, like creating a local function just to call the clsx but adding a description so it could be seen while hovering the function on the code or something like that
There was a problem hiding this comment.
we can do that in refactoring session.
There was a problem hiding this comment.
Yeah but if that can be done in refactoring season, ¿Why did you change this now? ¿Why did you add clsx function in this PR?
It's not a braking thing, the code will be as good as before but let's try to not do PR's with so many tasks because this particular change is not compleate/shouldn't be done now i think.
| <div className={styles.mazeContainer} ref={gameboardRef} tabIndex={0}> | ||
| <div className={styles.toolbar}> | ||
| {earnedButNotMintedCheddar > 0 && ( | ||
| {earnedButNotMintedCheddar > 100 && ( |
There was a problem hiding this comment.
I forgot to check this. Wait till i do plz.
There was a problem hiding this comment.
@Mart-Dan-Rossi did you remove the existing data from /db, because the structure of data changed after we added base, so maybe that might be the reason?
There was a problem hiding this comment.
There was a problem hiding this comment.
https://drive.google.com/file/d/1Ps9CdDtGCxzKrZTVn42GeyM0a-OF9Rqo/view?usp=sharing
Yeah, that's what i saw. I was thinking that if we can't make the button work properly BECAUSE we don't have the mint keys this functionallity is flat wong. In production users won't have mint keys.
There was a problem hiding this comment.
oh that's because the backend server has the keys, so the frontend user don't need it. It will work once we deploy the code to production.
There was a problem hiding this comment.
Hmmm.. And if it doesn't? I'm not sure how to handdle this situation. @RodrigoCSolari any suggestion?
There was a problem hiding this comment.
I guess, he can deploy it on testnet (the backend, which has the keys) and then we can test it.
15a6205 to
32af05f
Compare




Not able to recreated / removed from this PR