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

create OneOfEnumCell for select inputs #2414

Merged
merged 6 commits into from
Feb 24, 2025

Conversation

edkahara
Copy link
Contributor

@edkahara edkahara commented Jan 31, 2025

Context: https://jsonforms.discourse.group/t/oneof-not-producing-select-field-or-multiple-choice-checklist-in-vanilla-renderers/2574

  1. Create and export OneOfEnumCell cell that creates select inputs in vanella-renderers.
  2. Adds hideEmptyOption that when true, removes the empty option so the select input only has the options the user passes in oneOf

@CLAassistant
Copy link

CLAassistant commented Jan 31, 2025

CLA assistant check
All committers have signed the CLA.

Copy link

netlify bot commented Jan 31, 2025

Deploy Preview for jsonforms-examples failed.

Name Link
🔨 Latest commit 4229c93
🔍 Latest deploy log https://app.netlify.com/sites/jsonforms-examples/deploys/67b0a3a1f8bcdb0008ab5cc3

@lucas-koehler
Copy link
Contributor

Hi @edkahara , thank you for the contribution ❤️

Please note that you need to sign the contributor license agreement (CLA) before we can accept your contribution. See this comment: #2414 (comment)

@lucas-koehler lucas-koehler self-requested a review February 10, 2025 09:12
Copy link
Contributor

@lucas-koehler lucas-koehler left a comment

Choose a reason for hiding this comment

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

Hi @edkahara ,

Thanks again for the contribution! The code already looks pretty good to me. I just have one tiny suggestion for a comment.

Could you please add some tests for the new renderer? We try to have at least rudimentary unit tests for all renderers. You can use https://github.com/eclipsesource/jsonforms/blob/master/packages/vanilla-renderers/test/renderers/EnumCell.test.tsx as a starting point for this.

@edkahara
Copy link
Contributor Author

@lucas-koehler Is there any other change required?

@coveralls
Copy link

Coverage Status

coverage: 82.517% (+0.01%) from 82.504%
when pulling 4229c93 on edkahara:vanilla-renderers/select
into 903cef7 on eclipsesource:master.

Copy link
Contributor

@lucas-koehler lucas-koehler left a comment

Choose a reason for hiding this comment

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

Hi @edkahara thanks again for the contribution and adding these extensive tests ❤️

The code now looks good to me!
Building the example app fails here. However, this is not caused by this PR and building and testing it locally worked fine. Thus, we can still merge this :)

@lucas-koehler lucas-koehler merged commit 64fba38 into eclipsesource:master Feb 24, 2025
4 of 8 checks passed
@lucas-koehler lucas-koehler added this to the 3.6 milestone Mar 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants