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

🔧 vue 3 composition create pages #6453

Merged
merged 3 commits into from
Aug 2, 2023
Merged

Conversation

roiLeo
Copy link
Contributor

@roiLeo roiLeo commented Jul 21, 2023

⚠️ need to test /massmint page

edit:
massmint tabs doesn't work on beta

@roiLeo roiLeo requested a review from a team as a code owner July 21, 2023 13:15
@roiLeo roiLeo requested review from preschian and daiagi and removed request for a team July 21, 2023 13:15
@netlify
Copy link

netlify bot commented Jul 21, 2023

Deploy Preview for koda-canary ready!

Name Link
🔨 Latest commit 99241f2
🔍 Latest deploy log https://app.netlify.com/sites/koda-canary/deploys/64c77ab925740a0008a43a93
😎 Deploy Preview https://deploy-preview-6453--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 configuration.

@reviewpad
Copy link
Contributor

reviewpad bot commented Jul 21, 2023

AI-Generated Summary: This pull request features a refactoring of the Vue.js project structure. In general, there are alterations in the composables/useCreate.ts file as well as in several Vue components placed under the pages directory.

Firstly, within composables/useCreate.ts, the condition for setting the active tab has been tweaked with its matching numbers being adjusted accordingly.

In the files under the pages directory: bsx/create.vue, ksm/create.vue, rmrk/create.vue, snek/create.vue, and stmn/create.vue, the layout structure has been modified and the logic for showing explainer text has been changed. The usage of CreateMixin has been removed and refactored into a more compositional model using the useCreate composable.

Also, a mix-in file utils/mixins/createMixin.ts has been removed in the update, indicating a shift away from the mix-in approach in favor of more modern Vue 3 composition API practices.

Overall, these changes seem to provide more maintainable and readable code that aligns with Vue 3's composition design patterns.

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

reviewpad bot commented Jul 21, 2023

Reviewpad Report

⚠️ Warnings

  • Please link an issue to the pull request

@roiLeo roiLeo marked this pull request as draft July 21, 2023 13:25
@roiLeo roiLeo marked this pull request as ready for review July 21, 2023 13:36
@prury
Copy link
Member

prury commented Jul 21, 2023

@prury prury added the S-changes-requested-🤞 PR is almost good to go, just some fine tunning label Jul 21, 2023
Copy link
Contributor

@daiagi daiagi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code lgtm

@daiagi daiagi added the S-code-lgtm-✅ code review guild has reviewed this PR and it's code is approved label Jul 23, 2023
@codeclimate
Copy link

codeclimate bot commented Jul 31, 2023

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

View more on Code Climate.

@prury
Copy link
Member

prury commented Jul 31, 2023

RMRK2
Classic redirection: ❌
Massmint redirection:

RMRK1
Classic redirection: ❌
Simple redirection: ✔️
Massmint redirection: ❌

BSX
Classic redirection: ✔️
Massmint redirection: ✔️

SNEK
Classic redirection: ✔️
Massmint redirection: ✔️

STMN
Massmint Redirection: ✔️

will be updating as i go

@roiLeo
Copy link
Contributor Author

roiLeo commented Jul 31, 2023

Hello, just so you know I didn't change anything related to minting process.
Only thing changed are pages redirection & tab related with url (/:prefix/create/?tab=NFT & /:prefix/create/?tab=Collection)

@prury
Copy link
Member

prury commented Jul 31, 2023

Hello, just so you know I didn't change anything related to minting process. Only thing changed are pages redirection & tab related with url (/:prefix/create/?tab=NFT & /:prefix/create/?tab=Collection)

oh, thank you, will redirect my testing then

@roiLeo but i should also check if redirection works after i massmint, right?

@vikiival
Copy link
Member

Looks oki,
let's fix the redirect and happy to proceed

@prury
Copy link
Member

prury commented Aug 1, 2023

@roiLeo those RMRK redirections were already not working on beta tho

@roiLeo
Copy link
Contributor Author

roiLeo commented Aug 1, 2023

Wait does the redirection work for you? When I tested it on my side it was working as excepted (tab relates with URL path)

@prury
Copy link
Member

prury commented Aug 1, 2023

Wait does the redirection work for you? When I tested it on my side it was working as excepted (tab relates with URL path)

Oh, i got it now, yes , the tab redirection works properly for all of them. When testing i was testing both the tab redirection and the automatic redirection that happens after i mint something (this is what is not working)

I may have got confused because i saw a timeout function

@roiLeo
Copy link
Contributor Author

roiLeo commented Aug 1, 2023

Wait does the redirection work for you? When I tested it on my side it was working as excepted (tab relates with URL path)

Oh, i got it now, yes , the tab redirection works properly for all of them. When testing i was testing both the tab redirection and the automatic redirection that happens after i mint something (this is what is not working)

I may have got confused because i saw a timeout function

Alright I'll be checking this next week

@prury prury added S-works-for-me-✅ qa-guild has tested PR from end user perspective and functionality worked and removed S-changes-requested-🤞 PR is almost good to go, just some fine tunning labels Aug 1, 2023
@prury
Copy link
Member

prury commented Aug 1, 2023

Wait does the redirection work for you? When I tested it on my side it was working as excepted (tab relates with URL path)

Oh, i got it now, yes , the tab redirection works properly for all of them. When testing i was testing both the tab redirection and the automatic redirection that happens after i mint something (this is what is not working)

I may have got confused because i saw a timeout function

Alright I'll be checking this next week

Thank you for clarifying it for me

@yangwao yangwao merged commit 7ce5bf3 into kodadot:main Aug 2, 2023
19 checks passed
@roiLeo roiLeo deleted the chore/prefix/create branch August 6, 2023 12:13
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-code-lgtm-✅ code review guild has reviewed this PR and it's code is approved 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.

None yet

5 participants