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.

Sign up for GitHub

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

Jump to bottom

Add Object and OneOf renderer to vue3-vanilla #2151

Merged
merged 2 commits into from
Jul 7, 2023
Merged

Add Object and OneOf renderer to vue3-vanilla #2151

merged 2 commits into from
Jul 7, 2023

Conversation

butzist
Copy link
Contributor

@butzist butzist commented Jun 16, 2023

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

@netlify
Copy link

netlify bot commented Jun 16, 2023 •
edited
Loading

✅ 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
Copy link

CLAassistant commented Jun 16, 2023 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@sdirix
Copy link
Member

sdirix commented Jun 16, 2023

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
Copy link

coveralls commented Jun 16, 2023 •
edited
Loading

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
Copy link
Contributor Author

butzist commented Jun 17, 2023

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 reviewed Jun 20, 2023
View reviewed changes
Copy link
Member

@sdirix sdirix left a comment

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?

packages/vue/vue-vanilla/src/complex/ObjectRenderer.vue Outdated Show resolved Hide resolved
packages/vue/vue-vanilla/src/complex/OneOfRenderer.vue Outdated Show resolved Hide resolved
@butzist butzist force-pushed the feat/vue3-vanilla-object-and-oneof-renderer branch from ed9187e to a6a2f17 Compare June 21, 2023 15:18
@butzist
Copy link
Contributor Author

butzist commented Jun 21, 2023

@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
Copy link
Contributor Author

butzist commented Jun 21, 2023

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 reviewed Jun 27, 2023
View reviewed changes
Copy link
Contributor

@LukasBoll LukasBoll left a comment

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.

packages/vue/vue-vanilla/src/complex/OneOfRenderer.vue Outdated Show resolved Hide resolved
@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 approved these changes Jun 28, 2023
View reviewed changes
Copy link
Contributor

@LukasBoll LukasBoll left a comment

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 approved these changes Jul 7, 2023
View reviewed changes
Copy link
Member

@sdirix sdirix left a comment

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
@monodot monodot mentioned this pull request Nov 5, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LukasBoll LukasBoll LukasBoll approved these changes

@sdirix sdirix sdirix approved these changes

Assignees

@sdirix sdirix

@LukasBoll LukasBoll

Labels
None yet
Projects
None yet
Milestone
3.2
Development

Successfully merging this pull request may close these issues.
5 participants
@butzist @CLAassistant @sdirix @coveralls @LukasBoll