Skip to content

refactor: oruga composition api #8972

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

Closed
wants to merge 1 commit into from
Closed

Conversation

preschian
Copy link
Member

Thank you for your contribution to the KodaDot - One Stop Shop for Polkadot NFTs.

👇 __ Let's make a quick check before the contribution.

PR Type

  • Bugfix
  • Feature
  • Refactoring

Context

Copy link

netlify bot commented Jan 13, 2024

Deploy Preview for koda-canary ready!

Name Link
🔨 Latest commit 91114cb
🔍 Latest deploy log https://app.netlify.com/sites/koda-canary/deploys/65a1e94997a696000845ea61
😎 Deploy Preview https://deploy-preview-8972--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.

Copy link

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

View more on Code Climate.

Copy link

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

1 New issue
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link
Contributor

reviewpad bot commented Jan 13, 2024

AI-Generated Summary: This pull request primarily consists of refactoring done across various Vue and SCSS files. The bulk of the changes involve a transition from the Oruga UI library version oruga-next to a different version, theme-oruga. This is evident with numerous updates in import paths in SCSS files throughout the Vue components and stylesheets, signaling a potential shift or an update of the UI framework or theme being used in your project.

In terms of Vue components, there is a significant rework seen in NeoNotificationNotice.vue. This file revision demonstrates a systematic transformation of the component from a template-based architecture to a script setup block utilizing Vue's Composition API. This overhaul encompasses several new props and methods, updated computed component classes, and a substantial template update. It's suggested to carry out comprehensive testing for this component due to these extensive changes.

There are also changes spotted in NeoInputitems.vue and a newly created Vue component CheckRadioMixin.js which introduces a range of props, computed properties, watchers, and methods.

Additionally, updates are seen in posts.json where three post items have been added and three removed while pnpm-lock.yaml shows updates to specific package versions.

The only configuration change noticed was in nuxt.config.ts where the optimizeDeps configuration was removed. This might affect the optimization of dependencies in the development environment.

As SCSS files pathway changes are fairly common throughout, keep an eye out for any possible style deviations, observing closely the impact of additional new imports, _root.scss and _base.scss, across multiple files. One would also need to consider if there are any breaking changes between the two Oruga versions.

Barring few exceptions, the overall structure and logic of the code haven't been evidently altered. However, thorough testing and a detailed review would still be paramount due to the substantial number of files affected.

@reviewpad reviewpad bot added the large Pull request is large label Jan 13, 2024
Copy link
Contributor

reviewpad bot commented Jan 13, 2024

Reviewpad Report

⚠️ Warnings

  • Please link an issue to the pull request

@preschian preschian closed this Jan 13, 2024
@preschian preschian deleted the feat/oruga-composition-api branch January 13, 2024 02:03
@roiLeo
Copy link
Contributor

roiLeo commented Jan 16, 2024

no advantages in continuing to develop on this branch? is it the new composition api that causing all issue?

@preschian
Copy link
Member Author

no advantages in continuing to develop on this branch? is it the new composition api that causing all issue?

I couldn't make it. This is my 1st attempt to upgrade the Oruga package. On 2nd attempt locally, I tried to use multiple versions of Oruga and wanted to check it one by one. But the result is the same: the component does not render. I think the problem is we can't use extend: anymore because Oruga components are now using composition API.

I think the possible way is we need to "re-implement/re-import" (idk what the correct terms for this) the component again like the NeoButton currently used. But I don't like this approach, to be honest.

@preschian
Copy link
Member Author

It seems like the options for us are:

  1. Follow the NeoButton approach, "re-implement/re-import" the component. We need to re-create components using extends: or mixins: on @kodadot1/brick. Because we can't use extends: or mixins: anymore.
  2. Use Nuxt Layers. POC in here: feat: Nuxt Layers + Oruga #9020.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
large Pull request is large
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants