bcgov / bcrs-shared-components

BCRS Shared Components is a multi-package Lerna repository of shared Vue components, each published individually and that you can explore/ develop/ document/ test using Storybook.
https://bcgov.github.io/bcrs-shared-components/
Apache License 2.0
3 stars 30 forks source link

17118 update base address for vite #194

Closed JazzarKarim closed 1 year ago

JazzarKarim commented 1 year ago

Issue #: /bcgov/entity#17118

Description of changes:

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).

severinbeauvais commented 1 year ago

I thought all the tests passed previously?

JazzarKarim commented 1 year ago

I wouldn't approve all these changes on a Friday afternoon, but you're in tomorrow, right? :)

Sev, 99% of the changes here are from npm build storybook. The actual changes are at the end (only a handful).

severinbeauvais commented 1 year ago

PS It would be nice to separate the SB files from the other changes. 179 is a large number to review (even if I skip over most of them).

Actually, Travis noted recently that we should not commit the SB files, and the CI job should re-create them. We need to bug @pwei1018 to help us with this :)

JazzarKarim commented 1 year ago

I thought all the tests passed previously?

Yup, they do. I re-ran the job, all is green now.

JazzarKarim commented 1 year ago

PS It would be nice to separate the SB files from the other changes. 179 is a large number to review (even if I skip over most of them).

Actually, Travis noted recently that we should not commit the SB files, and the CI job should re-create them. We need to bug @pwei1018 to help us with this :)

Oh shoot. Shall I fix that? I'll only commit my changes?

JazzarKarim commented 1 year ago

@severinbeauvais Should be much much cleaner now. No more committed SB files.

severinbeauvais commented 1 year ago

PS It would be nice to separate the SB files from the other changes. 179 is a large number to review (even if I skip over most of them).

Actually, Travis noted recently that we should not commit the SB files, and the CI job should re-create them. We need to bug @pwei1018 to help us with this :)

Oh shoot. Shall I fix that? I'll only commit my changes?

We haven't implemented the auto-Storybook-build yet. See 17345.

For now, please commit the SB files.

severinbeauvais commented 1 year ago

I'm playing with this locally. It appears that validations are not working. I'm seeing this in the console.log when I make the address editable in SB for CompletingParty:

image

I tried adding "vuelidate-property-decorators" as a dependency for CompletingParty but that doesn't seem to help.

severinbeauvais commented 1 year ago

I tried adding "vuelidate-property-decorators" as a dependency for CompletingParty but that doesn't seem to help.

^^ This may or may not be needed but that's not the issue here.

The issue is with the way that validations are working. Previously, in the validation-mixin, this.$v[prop][key].$params.maxLength had a prop max with a value, but at the moment maxLength=null.

JazzarKarim commented 1 year ago

I tried adding "vuelidate-property-decorators" as a dependency for CompletingParty but that doesn't seem to help.

^^ This may or may not be needed but that's not the issue here.

The issue is with the way that validations are working. Previously, in the validation-mixin, this.$v[prop][key].$params.maxLength had a prop max with a value, but at the moment maxLength=null.

Sev, I just noticed that this issue has been present even before this PR. I went to the SB in the main repo (without my changes), and I can also see this error. main repo error

I was also having this issue in my Vite PR. I fixed it by playing with the vuelidate version. I'll check how things are going in here.

severinbeauvais commented 1 year ago

The issue is with the way that validations are working. Previously, in the validation-mixin, this.$v[prop][key].$params.maxLength had a prop max with a value, but at the moment maxLength=null.

We don't see this error in SB BaseAddress because no schema is declared.

With a schema:

image

JazzarKarim commented 1 year ago

^^ This may or may not be needed but that's not the issue here.

The issue is with the way that validations are working. Previously, in the validation-mixin, this.$v[prop][key].$params.maxLength had a prop max with a value, but at the moment maxLength=null.

Please try now Sev. Should be fine. The key is downgrade Vuelidate (https://github.com/bcgov/business-create-ui/pull/556/files#r1281230913).

severinbeauvais commented 1 year ago

Please try now Sev. Should be fine.

Indeed, now it works!

Can you please add an address schema to BaseAddress? You can prob use the same one as in the CompletingParty story.

And then we'll be good to merge this!

severinbeauvais commented 1 year ago
lerna notice
Successfully published:
 - @bcrs-shared-components/base-address@2.0.3
 - @bcrs-shared-components/completing-party@2.1.30
lerna success published 2 packages
JazzarKarim commented 1 year ago
lerna notice
Successfully published:
 - @bcrs-shared-components/base-address@2.0.3
 - @bcrs-shared-components/completing-party@2.1.30
lerna success published 2 packages

Thanks Sev!