Closed drehelis closed 1 year ago
Hi @autogun, thanks for the PR! I'll check it later
One question: what happens, if you change value in "Drop-A"? Will "Text-A" be updated?
But to be honest, I would prefer to go the way of #320 here, i.e. use "default" value rather than "values". For me, "default" option feels more natural.
Yes, Text-A
is updated accordingly to the Drop-A
selection.
By use default
, you mean from the configuration point of view?
To change this:
{
"name": "Text-A",
"values": {
"script": "echo ${Drop-A}_"
},
"required": true
},
to:
{
"name": "Text-A",
"default": {
"script": "echo ${Drop-A}_"
},
"required": true
},
Yes, exactly this one
How about consistency? Across all config, default
is key:value
and not an object.
While values are all objects with parameter injection ability.
Apparently, script
option is already supported there: #141 :sweat_smile:
But it doesn't support dependant parameters yet
As for me, the word default
reflects the behaviour better, than values
@bugy, one way to achieve this is in my last commit, but that still uses the infrastructure of values
.
This, however, allows me to use both default
and values
in a text config.
Hi @autogun, sorry for taking so long. To be honest, I would prefer to keep the code explicit, in this sense I would suggest to introduce explicit default_value_provider
, which would set a single default value.
Then you would need to extend model.parameter_config.ParameterModel._param_values_observer
to work with the default values provider
And create a method similar to model.parameter_config.ParameterModel._reload_values
, which would set default
On the client side, you would need to listen to "default" changes (not values)
When an old default value still equals to this.value
, then you can update it, by calling the following code:
this._doValidation(newValue);
this.$emit('input', newValue);
supported via "default" option in scope of #320. So closing this one as obsolete.
Following my embarrassing issue https://github.com/bugy/script-server/issues/442, trying to cherry-pick this change to hopefully merge it to master.
I'm not entirely sure the implementation is right thought. Meanwhile, this is what I got -
https://user-images.githubusercontent.com/11921235/119562769-65379500-bdaf-11eb-981b-f730d291ad82.mov