We currently have two distinct ways to provide custom widget options in uiSchema:
ui:widget directive,ui:options one.I'm in favor of using the latter, and deprecating support for the former. Thoughts?
Agreed, I have a few "wi-elds" that work with objects, arrays, base types. I like the ui:options structure a bit more myself. Can't really say why, in retrospect they both work pretty well.
I'm also in favor of the ui:options format. Few reasons to add:
PropTypes and therefore simplifies error detecting.Sorry for this dumb question, but I only see ui:options mentioned under Array items ordering.
Is exposing ui:options from the uiSchema to custom widgets a new feature in an upcoming release?
If yes, is the plan then to expose the uiSchema options to the custom widgets (through a new prop?) like this;
const uiSchema = {
"ui:options": {
"someOtherModel": someOtherModel,
"onUploadDone": Actions.addAsset,
},
images: {
items: {
guid: {
"ui:widget": {
"component": "customUploadWidget",
}
}
}
},
videos: {
items: {
guid: {
"ui:widget": {
"component": "customUploadWidget",
}
}
}
}
}
Instead of;
const uiSchema = {
images: {
items: {
guid: {
"ui:widget": {
options: {
"someOtherModel": someOtherModel,
"onUploadDone": Actions.addAsset,
},
"component": "customUploadWidget",
}
}
}
},
videos: {
items: {
guid: {
"ui:widget": {
options: {
"someOtherModel": someOtherModel,
"onUploadDone": Actions.addAsset,
},
"component": "customUploadWidget",
}
}
}
}
}
@matiaslarsson It would look like this:
const uiSchema = {
images: {
items: {
guid: {
"ui:widget": "customUploadWidget",
"ui:options: {
"someOtherModel": someOtherModel,
"onUploadDone": Actions.addAsset,
}
}
}
},
videos: {
items: {
guid:
"ui:widget": "customUploadWidget",
"ui:options: {
"someOtherModel": someOtherModel,
"onUploadDone": Actions.addAsset,
}
}
}
}
@olzraiti Got it, thanks!
So is this api design change to ui:options instead of ui:widget { options final? I would like to help making 0.41 final so I will look into this then. How about backwards compatibility?
I thought about it and I think I have a clean solution here with backwards compatibility.
So there are three ways to set options now, depending on which option you want to set:
ui:{optionName} (already used in production, quite good syntax)ui:options object (not yet used in production, but better syntax?)ui:widget.options object (already used in production, but awkward syntax).To align them all, a getUiOptions(uiSchema) function could be added in utils.js, which should be called in this way from each field for all options:
// in a Field
const {uiSchema} = this.props;
const {widget, barable, fooable} = {
widget: defaultWidget,
barable: true,
fooable: true,
...getUiOptions(uiSchema)
};
So the options syntax is abstracted away from the fields into one util function. This makes the field files cleaner and more consequent immediately.
In getUiOptions(), all 3 syntaxes could be accepted for every option, and for one or two syntaxes, a deprecation warning could be logged:
// in utils
export function getUiOptions(uiSchema) {
// get all passed options from ui:widget, ui:options, and ui:<optionName>
return Object.keys(uiSchema).filter(key => /^ui:/.test(key)).reduce((options, key) => {
const value = uiSchema[key];
if (key === "ui:widget" && typeof value === "object") {
// potential DEPRECATED warning can be called here
options["widget"] = value.component;
if (value.options) {
setOptions(options, value.options);
}
} else if (key === "ui:options" && typeof value === "object") {
// potential DEPRECATED warning can be called here
setOptions(options, value.options);
} else {
// potential DEPRECATED warning can be called here
options[key.substring(3)] = value;
}
return options;
}, {});
function setOptions(dest, src) {
Object.keys(src).forEach(option => {
dest[option] = src[option];
});
}
}
I would like to know what you think about this. I will PR if you think that this is the right approach.
@maartenth This indeed sounds like a very good plan, and I think that's the best compromise we could possibly come with in the shortest term :)
We'll probably have to officially deprecate the two other approaches and only document the ui:options one, and when tagging 1.0 we'll remove support for the old ways. Would that work for you folks? If so, I'll be happy to review a PR!
I will try to put together a PR in the next days. It is not trivial, I already saw this approach has some overlap with other util functions like getAlternativeWidget() and I need to understand what's going on in every field and widget with all the passing around of options.
First observation: there appears to be something wrong with current implementation of merging defaultProps.options of custom widgets with custom options from uiSchema, according to a test case I added in my branch. Custom options provided for one instance of a custom widget are applied to all instances of that custom widget, both when that widget is referenced as string and when it is referenced as function.
Two of the four tests fail; which tests fail, depends on the order of the fields...
Although my pull request #378 still has a test error, could someone already give some feedback on my changes? Especially for my question about the failing test. :) Thank you!
Most helpful comment
I thought about it and I think I have a clean solution here with backwards compatibility.
So there are three ways to set options now, depending on which option you want to set:
ui:{optionName}(already used in production, quite good syntax)ui:optionsobject (not yet used in production, but better syntax?)ui:widget.optionsobject (already used in production, but awkward syntax).To align them all, a
getUiOptions(uiSchema)function could be added inutils.js, which should be called in this way from each field for all options:So the options syntax is abstracted away from the fields into one util function. This makes the field files cleaner and more consequent immediately.
In
getUiOptions(), all 3 syntaxes could be accepted for every option, and for one or two syntaxes, a deprecation warning could be logged:I would like to know what you think about this. I will PR if you think that this is the right approach.