Control enhancements

[kchobantonov]

fixes #90

Allow allOf to also show properties if any, allow the object to display additionalProperties, patternProperties and conform with the propertyNames schema when defining the property names. The Add and Delete buttons will be disable when number of properties is greater or lower than what is specified in maxProperties/minProperties

Additional Properies Pattern Properties Property Names Size

[netlify[bot]]

✅ Deploy Preview for jsonforms-vuetify-renderers ready!

Name	Link
🔨 Latest commit	6d3315bcb34af7a350539ec0324b93470ac74f11
🔍 Latest deploy log	https://app.netlify.com/sites/jsonforms-vuetify-renderers/deploys/6392d8a3341deb00092886a3
😎 Deploy Preview	https://deploy-preview-91--jsonforms-vuetify-renderers.netlify.app/
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[kchobantonov]

@sdirix @lucas-koehler please review - demo is here

note that I have also included the css extraction part that was initially committed in webcomponent branch just in case that this PR gets merge and release before the webcomponent branch

[kchobantonov]

@sdirix will you have time soon to review this change and have a new release ?

[kchobantonov]

@sdirix @lucas-koehler does any of you have the time this week to review and merge this PR ?

[sdirix]

Hi @kchobantonov,

@lucas-koehler will review this PR this week. Depending on the number of required iterations this might then get in this week (or not). Once it's merged I can do a prerelease.

We love working on JSON Forms, but we only have limited time each month to handle topics which are not related to any of our customer projects. You are a very active community contributor, however the contributions are mostly limited to the Vuetify renderer set which is really only a minor part of the overall JSON Forms offerings. We therefore can't give full priority to all of your incoming contributions (at the very moment 4! large PRs simultaneously). Of course we will still look at them, but one at a time.

For the future we will try to communicate more clearly when and what we are looking next at. Once a reviewer is assigned to one of your PRs we look at it very soon. On your side you can try to polish the PRs as much as possible to help us out before requesting a review. You can use draft PRs if needed to mark that something is not yet ready to be reviewed. The smaller a PR is, the easier it's also for us to schedule a review. The less iterations and the less time reviewing your contributions take, the more we can potentially take on.

[kchobantonov]

@sdirix thanks for the feedback. I know that I'm pinging you frequently for particular PRs but that is only to make sure that I will get some response back that the PR will be assigned to someone for review and not wondering when this might be reviewed, and yes I'm aware of the draft PR but it happens that when such PR take a long time to be reviewed during that time I might find small issues here and there and because it is related to the same PR and this is still open I will reuse the PR to add that fix there as well instead of waiting for that to be merged and created another one.

Also thanks for the support and I'm aware that you are also busy with other stuff as well.

[kchobantonov]

@lucas-koehler thanks for the review - all suggestion that are now in the code as fixes I have marked those as resolve conversation - the only left here are those with the comments and why we need those. let me know if I can clarify some of those more.

[kchobantonov]

Hey @kchobantonov , I had a look at the code. See inline comments. Could you briefly explain why we need to extract the css because of the vue shadow dom? Also, I assume users would need to add this extracted css file to their index.html (or load it some other way), right?

I did not test the changes so far. I will do this later this week.

Yes they need to add the css the same way how the App.vue in the example application is adding that - e.g.

@import '~@jsonforms/vue2-vuetify/lib/jsonforms-vue2-vuetify.esm.css';

And this is needed because otherwise the inclusion of the CSS happens in the javascript code as it is using a default Vue CSS injector which injects the CSS on the document level which does not work when this is used as webcomponents. So it is better to be explicit what needs to be included where. Also all other major libraries have their CSS split so you can not only see how the CSS looks like but also could be used for CDN and etc.

[kchobantonov]

@kchobantonov Thanks you for the updates and explanations. I resolved the corresponding conversations. Please have a look at the unresolved ones (some of them were in the hidden ones when I opened the PR again).

I'm fine with the CSS extraction but please add information about this to the README at vue2-vuetify/README.me, e.g. in the code example. @sdirix Any objections to the CSS being extracted and bundled into a separate CSS file?

@kchobantonov Thanks you for the updates and explanations. I resolved the corresponding conversations. Please have a look at the unresolved ones (some of them were in the hidden ones when I opened the PR again).

I'm fine with the CSS extraction but please add information about this to the README at vue2-vuetify/README.me, e.g. in the code example. @sdirix Any objections to the CSS being extracted and bundled into a separate CSS file?

@lucas-koehler all suggested changes are included in the last commit

[kchobantonov]

@sdirix @lucas-koehler the only outstanding issue that I can see is with the Number and Integer controls where if for example for Number control we put a value for example 50.1234567890123441231313131313132 then in the actual JSON data we will have the number 50.123456789012344 which is correct but then the UI and the JSON data they do not exactly match. We have I guess 2 options here - either return the string if the value does not match with the String but that would be tricky since the parse number might be the same as number as the input string but written in exponent syntax (with E) or the second option is to synch the input with whatever we have successfully parsed as float - e.g. if parseFloat is truthy then to schedule an update for the UI so that the UI represent exactly what we have as data otherwise it looks strange... Note that if you paste the first number in the Number input the first time the number will be truncated to the second value but then you can still enter more numbers in the input

So which approach we should take ?

[lucas-koehler]

@kchobantonov Thanks for the update. There were actually 3 more comments from the first review. Sorry if I was not clear about that. I commented them again and tagged you so that you can find them.

the only outstanding issue that I can see is with the Number and Integer controls where if for example for Number control we put a value for example 50.1234567890123441231313131313132 then in the actual JSON data we will have the number 50.123456789012344 which is correct but then the UI and the JSON data they do not exactly match. We have I guess 2 options here - either return the string if the value does not match with the String but that would be tricky since the parse number might be the same as number as the input string but written in exponent syntax (with E) or the second option is to synch the input with whatever we have successfully parsed as float - e.g. if parseFloat is truthy then to schedule an update for the UI so that the UI represent exactly what we have as data otherwise it looks strange... Note that if you paste the first number in the Number input the first time the number will be truncated to the second value but then you can still enter more numbers in the input

I think we should go with the second approach: The UI should resemble the data as close as possible (aside from local specific formatting).

[kchobantonov]

@lucas-koehler I have removed for Number and Integer controls the type=number because it was working strange under chrome - for example when you enter - E1211 - or - 1.7976931348623157e+309 - which is larger than the max number 1.7976931348623157e+308 the string returned by the input was empty string - if I remove the type=number then it returns the actual string that is used.

Also I went with the case where if we can't convert to number then we pass the string and let the AJV handle the error that this is not a number. Since this is not input with type=number the control allows entering any characters so that is why we used not only parseFloat but also Number() parsing which is more strict but fails to return NaN for empty or strings with only whitespaces - that is why we use the combination of the two.

[lucas-koehler]

@kchobantonov Thanks for the updates again :) Your change for the numbers sound reasonable to me. I tested that an found one issue: If you enter a number that is too long. E.g. 5675.4444444444444444444444444444444 This then gets shortened to: 5675.444444444444 which is as we want it. When I now add another digit to the input field (e.g. a 5) the input shows 5675.4444444444445 but the data stays at 5675.444444444444. I guess what happens is that the UI doesn't get updated again because the data value stays the same. Could you have a look at that again?

[kchobantonov]

5675.4444444444444444444444444444444

it looks like similar issue is with the React renderers - although there there the issue is that the string representation does not match the actual number and no errors - e.g. it is just trimmed. So will make similar behavior - e.g. not sync the input with the number data value but will have a separate value to hold specifically the user input so that the string does not change based on what the number toString() representation returns - e.g. exponent vs. not exponent. I found also that the adaptValue function is not correct like this was the case for boolean when right now we can't set 0 - will adjust those shortly

Also in React renderers since the type=number is also there then if you enter E121 then the value is still blank but the input shows E121 - hence I have removed that from this render set but if the end user wants that he/she can use the vuetify options in the UI schema to pass that explicitly although in this case it will work strange.

Also in React if you enter very very long string with digits like

1212121212112312334534535435345354132345354334535435435435434534535435454354345354354313213239839058309583095830958309458309583098530958304583095830954132123123132132131312312312312312313213212313213213212312312312313213213213212312313212312313213131321132131231313131313131313132999999999999119191919191919191919191

then it does not show any errors - in this renderer we will show the error - just FYI - and also any number that we can parse successfully we will allow the AJV to determine if that is an integer or not - of course with many languages having some range restrictions on what integer is then we need to have min/max defined in the schema but any large number that is considered as integer by AJV will be allowed so that is why I have removed the parseInt while this is still used in React renderers - just wanted to point that out if you want to fix similar issues there.

[kchobantonov]

@lucas-koehler I have updated the integer and number controls to handle correctly 0 - there was again this issue with the truthy values like with the boolean controls. I have also make sure that we have a dedicated local data variable inputValue to hold what the user entered and the reason is that if we directly use the converted number then this number might get converted to a exponent form when the value is large and also the second reason is that we have removed the type=number by default (again the user can specify that in the UI schema as additional vuetify attributes if desired)

The integer control also have a new option - allowUnsafeInteger - that is boolean and by default is false which means that the integer number that we get needs to pass the check Number.isSafeInteger before converting it to a number. The reason for that is that when we type integers out of that range then the number that we type on the input can be different than the number stored in the data. The end users could opt out from that behavior if he/she specify allowUnsafeInteger to true to allow such integers.

I have removed the Number() call to validate additionally to the parseFloat if the value is valid and instead using regex to validate the input before converting it to number using parseFloat because of how parseFloat is implemented.

The issue that you have outlined above with 5675.4444444444444444444444444444444 should be fixes as well