Skip to content
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

✨ NeoMessage component #6068

Merged
merged 6 commits into from
May 26, 2023
Merged

✨ NeoMessage component #6068

merged 6 commits into from
May 26, 2023

Conversation

roiLeo
Copy link
Contributor

@roiLeo roiLeo commented May 22, 2023

PR Type

  • Refactoring

Context

Before submitting pull request, please make sure:

  • My contribution builds clean without any errors or warnings
  • I've merged recent default branch -- main and I've no conflicts
  • I've tried to respect high code quality standards
  • I've didn't break any original functionality

Screenshot 📸

Screenshot 2023-05-22 at 17-13-24 KodaDot Low fees and low carbon minting KodaDot - NFT Market Explorer

Copilot Summary

🤖 Generated by Copilot at f92b398

Refactored the token creation feature to use a custom NeoMessage component from the @kodadot1/brick library instead of the Buefy b-message component. Deleted unused or redundant components and added documentation and testing for the new component.

🤖 Generated by Copilot at f92b398

Sing, O Muse, of the skillful refactorer
Who from the Buefy b-message component
Crafted a new and splendid NeoMessage
And placed it in the @kodadot1/brick library.

@roiLeo roiLeo requested a review from a team as a code owner May 22, 2023 15:21
@roiLeo roiLeo requested review from preschian and vikiival and removed request for a team May 22, 2023 15:21
@netlify
Copy link

netlify bot commented May 22, 2023

Deploy Preview for koda-canary ready!

Name Link
🔨 Latest commit 854278d
🔍 Latest deploy log https://app.netlify.com/sites/koda-canary/deploys/64706d724d2bc9000702f543
😎 Deploy Preview https://deploy-preview-6068--koda-canary.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@reviewpad reviewpad bot added medium Pull request is medium waiting-for-review labels May 22, 2023
@reviewpad
Copy link
Contributor

reviewpad bot commented May 22, 2023

AI-Generated Summary: This pull request includes four patches related to the removal of unused components and computed properties in the CreateToken.vue file. New additions include a NeoMessage component, modifications to the MessageNotify.vue file, and the removal of JustMintedTokenMessage.vue file. Other changes consist of removing the unused NeoMessage component, removing the hasPrice computed property, and cleaning up the code.

@vikiival
Copy link
Member

Copy-pasted logic from b-message ?

@roiLeo
Copy link
Contributor Author

roiLeo commented May 22, 2023

Copy-pasted logic from b-message ?

Oui.

@vikiival
Copy link
Member

Oui.

Error while parsing: Viki parser is unable to decode emotional context behind the message

@roiLeo
Copy link
Contributor Author

roiLeo commented May 22, 2023

Oui.

Error while parsing: Viki parser is unable to decode emotional context behind the message

Low quality effort, it's working but at what cost?

@roiLeo roiLeo mentioned this pull request May 24, 2023
37 tasks
@prury
Copy link
Member

prury commented May 24, 2023

seems to be working fine in the deployed version, but i wasn't able to reproduce the error using the link provided on #5683, @roiLeo can you provide more screenshots of what has been changed or it was just this specific message?

@roiLeo
Copy link
Contributor Author

roiLeo commented May 25, 2023

seems to be working fine in the deployed version, but i wasn't able to reproduce the error using the link provided on #5683

I don't know maybe it was fixed in another PR, I've just render component in random pages to see if it was working.

@roiLeo can you provide more screenshots of what has been changed or it was just this specific message?

In this PR you can test the NeoMessage by adding ?congratsNft=congrats to any Nft Item

example:
https://deploy-preview-6068--koda-canary.netlify.app/bsx/gallery/4104196707-39?congratsNft=congrats

Note: I know it's not looking good, this is a Refactoring PR no design changes has been made.

@prury
Copy link
Member

prury commented May 25, 2023

seems to be working fine in the deployed version, but i wasn't able to reproduce the error using the link provided on #5683

I don't know maybe it was fixed in another PR, I've just render component in random pages to see if it was working.

@roiLeo can you provide more screenshots of what has been changed or it was just this specific message?

In this PR you can test the NeoMessage by adding ?congratsNft=congrats to any Nft Item

example: https://deploy-preview-6068--koda-canary.netlify.app/bsx/gallery/4104196707-39?congratsNft=congrats

Note: I know it's not looking good, this is a Refactoring PR no design changes has been made.

Thank you!
What event should trigger this message in the future? a NFT mint?

Apart from the design side then, i found one bug on mobile(resized browser), if you click on the QR code button and leave it open for long enough for the timer to hide the modal, it locks the page scroll.

@roiLeo
Copy link
Contributor Author

roiLeo commented May 25, 2023

Thank you! What event should trigger this message in the future? a NFT mint?

Yes! keep in mind this is going to change in (can't find realted issue edit: #5458)

Apart from the design side then, i found one bug on mobile(resized browser), if you click on the QR code button and leave it open for long enough for the timer to hide the modal, it locks the page scroll.

if you're able to reproduce it on this preview and beta, then we can create a new issue.
If it's only comming from my changes, I can take a look.

@prury
Copy link
Member

prury commented May 25, 2023

Thank you! What event should trigger this message in the future? a NFT mint?

Yes! keep in mind this is going to change in (can't find realted issue)

Apart from the design side then, i found one bug on mobile(resized browser), if you click on the QR code button and leave it open for long enough for the timer to hide the modal, it locks the page scroll.

if you're able to reproduce it on this preview and beta, then we can create a new issue. If it's only comming from my changes, I can take a look.

aww its also happening in beta, I'll open a new issue then

edit: #6107

@prury prury added the S-works-for-me-✅ qa-guild has tested PR from end user perspective and functionality worked label May 25, 2023
@codeclimate
Copy link

codeclimate bot commented May 25, 2023

Code Climate has analyzed commit 6478859 and detected 0 issues on this pull request.

View more on Code Climate.

@roiLeo roiLeo merged commit 511643c into kodadot:main May 26, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
medium Pull request is medium S-works-for-me-✅ qa-guild has tested PR from end user perspective and functionality worked waiting-for-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rmrk minting some dialog
5 participants