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
In relation to issue #216, this patch solves the following problems: #218
base: master
Are you sure you want to change the base?
Conversation
1) v-combobox is used instead of v-select when explicitly requested by "x-display" or when more than 20 options are present 2) a new selection type element is introduced - v-chip-group with v-chip items 3) a bug in FileProperty.js was fixed ("writeOnly" can exist not only on the top-level field definition but also inside "items" when the type is "array") 4) fixed a bug in the test definition for if/then/else conditionals - neither Ajv is used/defined nor a plain validation function is provided 5) fixed a bug in the prefilled-arrays.js test definition - the default icon "map-marker" does not have a matching mockup URL 6) fixed the warning from VSlider about missing v-app in the test suite 7) updated the version of @nuxtjs/vuetify - v1 requires fibers which needs node-gyp which needs Visual Studio on Windows (solving issue nuxt-community/vuetify-module#254) 8) updated the test snapshots
A large part of this is ok. But I don't think it is ok to simply replace autocomplete with combobox. If I am not mistaken this will break the current behavior in many use case, and the form will allow the user to input an invalid model (in case of select values coming from enum or oneOf). I think combobox should be available as an option in case of a list of values filled with x-fromData of x-fromUrl, but not from enum or oneOf. There is a pull-request (that I totally forgot to merge :() to fill select values with the examples annotation, in this case combobox is the appropriate component. Also, could you complete one of the example in the doc to illustrate the new chip-group display ? |
I have fixed the code so that it uses |
Thanks. Regarding the comment "Unfortunately the `v-chip-group` Vuetify component does not support label and prepend/append slots at the moment - so no field title or help tooltip will be rendered.". I think this could be solved by wrapping the v-chip-group inside a standard v-input. This is what I do in wrapper components for third party libs (see this example). This way you can have labels, validation rules, etc on any kind of input. |
IS this tip-tap wrapper available for using from consumer applications - or it is just for the VJSF documentation? |
See this example. For the time being the tiptap wrapper is just an example that you can imitate. In the near future I intend to maintain a small number of wrapper components and encourage other developpers to do the same thing. |
If I understand you correctly - you encourage me to use the tip-tap wrapper in the |
I was simply referring to the tiptap wrapper as an example of using v-input around another component in order to create uniformity of labels, etc. But the comparison stops here. Tiptap is a heavy third-party lib, this is why I do not integrate it directly in vjsf and instead create a separate component that users can load separately if they want to. v-chip-group is part of vuetify, I have no problem with using it directly in vjsf as you did in your commit (but probably with a v-input around it). |
If I update my PR to reflect your suggestion (i.e. wrap |
I think vuetify will not break compatibility and the component will keep the same rendering for the same given prop. But additional props might be supported. So you should not pass the props that are ignored for now (label, etc), this way if they are not ignored in the future the behavior will not change. |
Actually a better example is the color-picker. |
v-combobox
is used instead ofv-select
when explicitly requested by x-display or when more than 20 options are presentv-chip-group
withv-chip
itemsFileProperty.js
was fixed (writeOnly can exist not only on the top-level field definition but also inside items when the type is array)if/then/else
conditionals - neitherAjv
is used/defined nor a plain validation function is providedprefilled-arrays.js
test definition - the default icon map-marker does not have a matching mockup URLVSlider
about missingv-app
in the test suite@nuxtjs/vuetify
- v1 requires fibers which needs node-gyp which needs Visual Studio on Windows (solving issue Make fibers an optional dependency nuxt-community/vuetify-module#254)