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

initial contribution for vue-vuetify renderer set #2341

Open
wants to merge 25 commits into
base: master
Choose a base branch
from

Conversation

kchobantonov
Copy link
Contributor

@sdirix please review the initial contribution that have the changes that fixes the vue3 integration. Few things to note:

  1. The original example data in the vuetify render set was not migrated since we do have an example set with the exceptions for few examples ported - additional properties, login form and etc. The only issue here is that the original examples were having the vuetify UI options that were not included or ported.
  2. few minor fixed in the core (issues with undefined in the core)
  3. upgrade of ajv-i18n since the Vite do not like the require when expecting to use import

Copy link

netlify bot commented May 30, 2024

Deploy Preview for jsonforms-examples ready!

Name Link
🔨 Latest commit fccbce5
🔍 Latest deploy log https://app.netlify.com/sites/jsonforms-examples/deploys/666aa49f6900f10008513c1a
😎 Deploy Preview https://deploy-preview-2341--jsonforms-examples.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.

@kchobantonov
Copy link
Contributor Author

@sdirix can you review and check why we have out of memory failure on the build machine ? Perhaps we need a build machine with more memory ?

@kchobantonov kchobantonov changed the title intiial contribution for vue-vuetify renderer set initial contribution for vue-vuetify renderer set May 30, 2024
@kchobantonov
Copy link
Contributor Author

@sdirix the build is fixed - I had to change

    "build-only": "vite build",

to

    "build-only": "NODE_OPTIONS=\"--max-old-space-size=4096\" vite build",

in the vuetiy package - if you want you can remove that from there and place it in the ci.yaml the same way how we set that for test to be set on build as well but that is up to you.

@sdirix
Copy link
Member

sdirix commented Jun 6, 2024

Thanks! I plan to take a look at this PR next week.

I was wondering whether we should preserve the history of the other repository, i.e. instead of copying all files over to here, we can add the vue3-branch into this repository as an orphan-branch and then do a merge commit with the content of the first commit here. This would keep the history.

Suggestion: Once this PR here is in a good state and reviewed, I can do as suggested above with your contribution here as a commit on top.

Note that we currently lose the new features we recently introduced in the vue3 branch, especially the touch-error filtering (eclipsesource/jsonforms-vuetify-renderers#107), but also the improved exports (eclipsesource/jsonforms-vuetify-renderers#109 & eclipsesource/jsonforms-vuetify-renderers#110)

Can you re-integrate those changes?

@coveralls
Copy link

Coverage Status

coverage: 83.346% (+0.02%) from 83.322%
when pulling 77f2714 on kchobantonov:vue-vuetify
into 52aa15a on eclipsesource:master.

@kchobantonov
Copy link
Contributor Author

I can review and apply those changes - keep in mind that this PR will have almost the same thing as the PR for fixing the vue3 without the recent changes (few weeks ago) that were merged into that repo, and also this PR different from the PR against the older vue 3 vuetify 3 repository since I had to integrate that with the jsonforms main repo and most of the changes are in the example so both example apps are different between this PR and the other one. If you want to keep the history please create the proper PR with the history from the other repo and the either I or you can override that on top and have the history there.

@kchobantonov
Copy link
Contributor Author

@sdirix the requested PRs are applied to this PR

@sdirix
Copy link
Member

sdirix commented Jun 7, 2024

@kchobantonov Thank you very much!

@coveralls
Copy link

Coverage Status

coverage: 83.346% (+0.02%) from 83.322%
when pulling 406ecc1 on kchobantonov:vue-vuetify
into 52aa15a on eclipsesource:master.

@sdirix
Copy link
Member

sdirix commented Jun 14, 2024

I'm looking at the PR. I plan to contribute the migration and finish the review next week.

@coveralls
Copy link

Coverage Status

coverage: 83.359% (+0.02%) from 83.337%
when pulling fccbce5 on kchobantonov:vue-vuetify
into c00b664 on eclipsesource:master.

@sdirix sdirix added this to the 3.4 milestone Jun 19, 2024
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.

None yet

4 participants