Skip to content

Add Object and OneOf renderer to vue3-vanilla - #2151

Merged
sdirix merged 2 commits into
eclipsesource:masterfrom
DataHow:feat/vue3-vanilla-object-and-oneof-renderer
Jul 7, 2023
Merged

Add Object and OneOf renderer to vue3-vanilla#2151
sdirix merged 2 commits into
eclipsesource:masterfrom
DataHow:feat/vue3-vanilla-object-and-oneof-renderer

Conversation

@butzist

@butzist butzist commented Jun 16, 2023

Copy link
Copy Markdown
Contributor

We ported the Object and OneOf renderers from vue2 to vue3 - would be nice to have them upstream

@netlify

netlify Bot commented Jun 16, 2023

Copy link
Copy Markdown

Deploy Preview for jsonforms-examples ready!

Name Link
🔨 Latest commit 356347b
🔍 Latest deploy log https://app.netlify.com/sites/jsonforms-examples/deploys/649afcd52990b10008ebb1d7
😎 Deploy Preview https://deploy-preview-2151--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.

@CLAassistant

CLAassistant commented Jun 16, 2023

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@sdirix

sdirix commented Jun 16, 2023

Copy link
Copy Markdown
Member

Thanks for the contribution ❤️

In general we are definitely interested in these renderers 👍 . Please make sure that the build succeeds (there seem to be some Typescript errors) and sign the CLA. Afterwards we'll take a look. Thanks!

@butzist
butzist force-pushed the feat/vue3-vanilla-object-and-oneof-renderer branch from f5a4dd1 to 695ea4f Compare June 16, 2023 15:41
@coveralls

coveralls commented Jun 16, 2023

Copy link
Copy Markdown

Coverage Status

coverage: 84.254%. remained the same when pulling 356347b on DataHow:feat/vue3-vanilla-object-and-oneof-renderer into 7e0115f on eclipsesource:master.

@butzist
butzist force-pushed the feat/vue3-vanilla-object-and-oneof-renderer branch from 695ea4f to ed9187e Compare June 17, 2023 06:30
@butzist

butzist commented Jun 17, 2023

Copy link
Copy Markdown
Contributor Author

Update: added some tests for the new renderers, but was not able to test the dialog in the OneOfRenderer (showModal() is not a function). It worked in manual testing though. Let me know if there is anything missing.

@sdirix sdirix left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good in general! Can you take a look at my comments?

Comment thread packages/vue/vue-vanilla/src/complex/ObjectRenderer.vue Outdated
Comment thread packages/vue/vue-vanilla/src/complex/OneOfRenderer.vue Outdated
@butzist
butzist force-pushed the feat/vue3-vanilla-object-and-oneof-renderer branch from ed9187e to a6a2f17 Compare June 21, 2023 15:18
@butzist

butzist commented Jun 21, 2023

Copy link
Copy Markdown
Contributor Author

@sdirix Thank you for the review. I tried to adress them as good as I can and also fixed linting.

@butzist
butzist force-pushed the feat/vue3-vanilla-object-and-oneof-renderer branch 2 times, most recently from a291594 to ea7a5cd Compare June 21, 2023 15:50
@butzist

butzist commented Jun 21, 2023

Copy link
Copy Markdown
Contributor Author

Now also the build is passing for me - sorry for the inconvenience, but had some trouble getting the build system to work locally

@butzist
butzist force-pushed the feat/vue3-vanilla-object-and-oneof-renderer branch from ea7a5cd to 71dd23f Compare June 22, 2023 05:55

@LukasBoll LukasBoll left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you for the great contribution @butzist!
Everything looks good to me. I just found one issue, which I addressed in my comment below.

Comment thread packages/vue/vue-vanilla/src/complex/OneOfRenderer.vue Outdated
@butzist
butzist force-pushed the feat/vue3-vanilla-object-and-oneof-renderer branch from 71dd23f to 356347b Compare June 27, 2023 15:14

@LukasBoll LukasBoll left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me!

@sdirix sdirix left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Very nice! Thank you again for the contribution! ❤️

@sdirix
sdirix merged commit f74e9ed into eclipsesource:master Jul 7, 2023
@sdirix sdirix added this to the 3.2 milestone Jul 7, 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.

5 participants