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

17124 + 16550 - Vue3 upgrade: Upgrade a component + Storybook #204

Merged
merged 3 commits into from
Nov 17, 2023

Conversation

ketaki-deodhar
Copy link
Collaborator

@ketaki-deodhar ketaki-deodhar commented Nov 9, 2023

Issue #: /bcgov/entity#17124 and /bcgov/entity#16550

Description of changes:

  • Upgraded Vue3, Vuetify3 and related libraries (ESLint, Vite, Vuetify, vuelidate etc.)

    • Add vuetify.ts file
    • Updated preview.ts file to use vuetify (new way of using Vuetify)
  • Upgraded DetailComment component to work with Vue3 and Vuetify3 (refer Vue3 and Vuetify3 guide)

    • Modified component to work with Vuetify3 e.g. <v-textarea> attributes are a bit different in Vuetify3. Each component will be distinct case
    • Replaced vue-property-decorator with vue-facing-decorator (devDependencies section) and updated Vue version in package.json file for individual component
  • Upgraded Storybook

    • Upgraded Storybook to 7.5.3 and related packages to work with Vue3
    • Modified *.stories.ts file for a component to make it work with new version of Storybook and Vue3
  • Still working on document/notes explaining how to start working on upgrading other components. Will add some references and links in the doc

After upgrade -

2023-11-09.15.16.16.mp4

DEV link for comparison - https://bcgov.github.io/bcrs-shared-components/

Notes:

  • Developers will see some errors while running storybook. Need to look at those when specific component is being worked on.
  • npm run lint lists all lint errors

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the business-filings-ui license (Apache 2.0).

@ketaki-deodhar ketaki-deodhar self-assigned this Nov 9, 2023
@@ -7,10 +7,6 @@ module.exports = {
extends: [
'eslint:recommended',
'plugin:@typescript-eslint/recommended',
'plugin:vue/base',
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed Vue2 specific plugins

Copy link
Collaborator

Choose a reason for hiding this comment

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

Are these for Vue2 only, or did they bring value for Vue3 code?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Are there updated plugins for Vuetify 3?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I read the documentation again vue/base should be included. No updates for Vuetify 3

Copy link
Collaborator

Choose a reason for hiding this comment

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

Perfect, thanks!

@@ -34,8 +34,7 @@
</template>

<script lang="ts">
import Vue from 'vue'
import { Component, Prop } from 'vue-property-decorator'
import { Component, Prop, Vue } from 'vue-facing-decorator'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For Vue3, vue-property-decorator is replaced with vue-facing-decorator. This change is done in all the components to avoid all the console errors.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I support this. Thanks!

public validations (): any {
return { addressLocal: { ...this.schemaLocal } }
}
// @Validations()
Copy link
Collaborator Author

@ketaki-deodhar ketaki-deodhar Nov 9, 2023

Choose a reason for hiding this comment

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

As per comment above (in the code) this needs to be looked at when we tackle this component.

@@ -0,0 +1,22 @@
// Styles
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

vuetify.ts file added as per Vuetify3 guide

]
}
setup((app:App) => {
app.use(vuetify)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

New way of using vuetify ;)

@ketaki-deodhar ketaki-deodhar marked this pull request as ready for review November 9, 2023 23:27
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@severinbeauvais
Copy link
Collaborator

Amazing work, Ketaki! Just a few questions above.

@ketaki-deodhar
Copy link
Collaborator Author

Amazing work, Ketaki! Just a few questions above.

I will look at all the amazing suggestions and questions today

Copy link
Collaborator

@JazzarKarim JazzarKarim left a comment

Choose a reason for hiding this comment

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

LGTM! Awesome stuff Ketaki! 👍

Copy link
Collaborator

@severinbeauvais severinbeauvais left a comment

Choose a reason for hiding this comment

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

I just have that question about addon-essentials, otherwise good to go -- merge when ready.

Edit: question was resolved.

@ketaki-deodhar ketaki-deodhar merged commit b02734d into feature-vue3 Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants