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

Vue: Disable ArrayListRenderer add if control.data.length exceeds or is equal to maxItems #2154

Merged

Conversation

Droxx
Copy link
Contributor

@Droxx Droxx commented Jul 4, 2023

The included ArrayListRenderer in Vue-Vanilla would allow more items to be added to an array than the schema allowed.

This PR uses the maxItems value from the schema (if present) and disables the add button if the control.data array is an equal, or greater, length to the maxItems value in the schema

@netlify
Copy link

netlify bot commented Jul 4, 2023

Deploy Preview for jsonforms-examples ready!

Name Link
🔨 Latest commit 274854e
🔍 Latest deploy log https://app.netlify.com/sites/jsonforms-examples/deploys/64ac2860dd09f4000774cfaa
😎 Deploy Preview https://deploy-preview-2154--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 Jul 4, 2023

CLA assistant check
All committers have signed the CLA.

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.

Thanks for the contribution ❤️. Much appreciated.

Please check the comments and add tests for the new use case(s). Also the build must succeed, it seems the linter is complaining.

packages/vue/vue-vanilla/src/array/ArrayListRenderer.vue Outdated Show resolved Hide resolved
packages/vue/vue-vanilla/src/array/ArrayListRenderer.vue Outdated Show resolved Hide resolved
@sdirix sdirix added this to the 3.2 milestone Jul 7, 2023
@Droxx
Copy link
Contributor Author

Droxx commented Jul 10, 2023

Thanks for the feedback @sdirix. I have also implemented minItems checking on the delete button.

I've left one comment on your review. But besides this it should be ready for re-evaluation

@Droxx Droxx requested a review from sdirix July 10, 2023 09:02
@coveralls
Copy link

coveralls commented Jul 10, 2023

Coverage Status

coverage: 84.254%. remained the same when pulling 274854e on Droxx:disable-array-add-if-count-exceeds-maxitems into 7e0115f on eclipsesource:master.

@sdirix
Copy link
Member

sdirix commented Jul 10, 2023

I cannot see this check being performed on any other renderers in the project (I do see it in use in the vuetify renderers). So I left it out in order to remain consistent with the other renderers

Besides the Vuetify renderers (ListWithDetail, ArrayControlRenderer, AdditionalProperties, AnyOfStringOrEnumControlRenderer, MultiStringControlRenderer, PasswordControlRenderer, StringControlRenderer, StringMaskControlRenderer, ArrayLayoutRenderer), restrict is used in various string and number renderers, see MaterialAnyOfStringOrEnumControl, MuiInputNumberFormat, MuiInputText, NumberFormatCell, TextCell.

I cannot find documentation on how to set this option when using json-forms in vue. If you could point me in this direction, it would be helpful!

You can set it in each Ui Schema options, e.g.

{
  type: 'Control',
  scope: '#/properties/name',
  options: {
    restrict: true
  }
}

Or globally via the config

<json-forms config="config"/>

setup(){
  return ({
    config: { restrict: true}
  });
}

package-lock.json Outdated Show resolved Hide resolved
@Droxx
Copy link
Contributor Author

Droxx commented Jul 10, 2023

@sdirix Thanks for the comments about the appliedOptions.restrict. I have added this to the control.

I have also addressed your latest points

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.

Thanks for the contribution ❤️

@sdirix sdirix merged commit 369370a into eclipsesource:master Jul 11, 2023
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