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

🔧 vue3 composition api pages #6379

Merged
merged 5 commits into from
Jul 4, 2023
Merged

Conversation

roiLeo
Copy link
Contributor

@roiLeo roiLeo commented Jul 3, 2023

part of #4750

  • remove ExperimentMixin
  • vue3 composition /:prefix/collection/:id/activity page
  • vue3 composition /:prefix/explore/items page
  • vue3 composition /:prefix/explore/collectibles page
  • vue3 composition /identity page
  • vue3 composition /sustainibility page

edit: DRAFT doesn't work yet

@roiLeo roiLeo requested a review from a team as a code owner July 3, 2023 14:29
@roiLeo roiLeo requested review from vikiival and daiagi and removed request for a team July 3, 2023 14:29
@netlify
Copy link

netlify bot commented Jul 3, 2023

Deploy Preview for koda-canary ready!

Name Link
🔨 Latest commit 331efd7
🔍 Latest deploy log https://app.netlify.com/sites/koda-canary/deploys/64a2e5251a6a8a00086a9d00
😎 Deploy Preview https://deploy-preview-6379--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 3, 2023

AI-Generated Summary: This pull request primarily revolves around the refactoring of Vue.js components in order to utilize Vue 3's Composition API. It involves modifications in the 'activity.vue' and 'items.vue' files located inside the '/pages/_prefix' directory. These changes involved removing the previous usage of Vue Class components syntax, replacing them with the Composition API's syntax to improve readability and maintainability of the code base.

Furthermore, one test case in 'media.cy.ts' was updated to remove a query parameter from the visit URL, implying a move away from dynamic testing based on this parameter.

Finally, the mixin 'experimentMixin.ts' was removed, signaling that the functionality it provided (deciding whether to test a redesigned version of the site) has been deprecated or replaced elsewhere in the code.

Overall, this suggests a shift towards consolidating functionality and making the code more direct and modern, following best practices for using Vue 3. The use of Composition API provides better re-usability, organization, and understanding of the code by developers. As with all refactors, proper testing is advised to ensure all functionalities work as expected after the modifications.

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

reviewpad bot commented Jul 3, 2023

Reviewpad Report

⚠️ Warnings

  • Please link an issue to the pull request

@roiLeo roiLeo changed the title 🔧 vue3 composition api explore 🔧 vue3 composition api pages Jul 3, 2023
@roiLeo roiLeo marked this pull request as draft July 3, 2023 14:59
@roiLeo roiLeo marked this pull request as ready for review July 3, 2023 15:11
@codeclimate
Copy link

codeclimate bot commented Jul 3, 2023

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

View more on Code Climate.

@vikiival
Copy link
Member

vikiival commented Jul 3, 2023

Looks good sir

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

prury commented Jul 3, 2023

did not find any strange behavior or different console warnings compared to canary on those pages

@prury
Copy link
Member

prury commented Jul 3, 2023

As suggested by @roiLeo, i'm testing share preview on the modified pages

Problems encountered so far(telegram, discord):
https://deploy-preview-6379--koda-canary.netlify.app/bsx/collection/1194754914/activity
image

/sustainability works now, wasn't working on canary
/collectibles works now, wasn't working on canary
/items works now, wasn't working on canary
/identity works now, wasn't working on canary

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.

lgtm

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

roiLeo commented Jul 4, 2023

As suggested by @roiLeo, i'm testing share preview on the modified pages

Problems encountered so far(telegram, discord): https://deploy-preview-6379--koda-canary.netlify.app/bsx/collection/1194754914/activity image

I've made some test and it's not related with this PR, preview doesn't work for url starting with deploy-preview-...
Tested on https://deploy-preview-6380--koda-canary.netlify.app/bsx/collection/1194754914/activity

Capture d’écran 2023-07-04 à 8 37 16 AM

@vikiival vikiival merged commit e4868ee into kodadot:main Jul 4, 2023
19 checks passed
@roiLeo roiLeo deleted the chore/refact/explore branch July 4, 2023 08:02
@prury
Copy link
Member

prury commented Jul 4, 2023

I've made some test and it's not related with this PR, preview doesn't work for url starting with deploy-preview-... Tested on https://deploy-preview-6380--koda-canary.netlify.app/bsx/collection/1194754914/activity

Good to know! thank you

@roiLeo
Copy link
Contributor Author

roiLeo commented Jul 4, 2023

works well now
Capture d’écran 2023-07-04 à 2 37 29 PM

This was referenced Jul 5, 2023
This was referenced Jul 17, 2023
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

4 participants