SequenceParam 'required' flag ends up redundant with much code in validate() method. #56
Labels
enhancement
New feature or request
Comments
gwhitney
added a commit
to gwhitney/frontscope
that referenced
this issue
Feb 15, 2022
To allow params to be of any type (rather than just `string | number | boolean` and to allow params to correspond to correctly-typed top-level properties, this commit creates a common base class Paramable for Sequences and Visualizers that handles communication of params with the UI. Basically, a class just derives from Paramable, defines a `params` property with objects describing the top-level properties that should be params, and defines a checkParameters() function that determines if a collection of candidate values (as `value` properties in the `params` object) are acceptable. If so, they are copied automatically into the implementing object's top-level properties. If the implementation changes any of these properties and wants the change to be reflected in the UI, it must call `this.refreshProperties()` manually. Also, extends SeqVizParamModal.vue to present reasonable UI interfaces for a much larger variety of param types, as well as adds features such as hidable params, indentation, and so on. Resolves numberscope#55. Resolves numberscope#56. Resolves numberscope#60. Resolves numberscope#62. Resolves numberscope#82. (Although the graphic presentation for collapsible subgroups could be more polished.) Also takes care of numberscope#45 in all sequences and visualizers (there are still a few gratuitous console.logs in components, though). Lays groundwork for numberscope#68, in that the channel for communication between the UI and the sequence/visualizer is now very clear and as close to direct as possible (I don't think we want it any more direct; if it were, then in the midst of typing the visualizer could be left in an invalid state.)
gwhitney
added a commit
to gwhitney/frontscope
that referenced
this issue
Feb 15, 2022
To allow params to be of any type (rather than just `string | number | boolean` and to allow params to correspond to correctly-typed top-level properties, this commit creates a common base class Paramable for Sequences and Visualizers that handles communication of params with the UI. Basically, a class just derives from Paramable, defines a `params` property with objects describing the top-level properties that should be params, and defines a checkParameters() function that determines if a collection of candidate values (as `value` properties in the `params` object) are acceptable. If so, they are copied automatically into the implementing object's top-level properties. If the implementation changes any of these properties and wants the change to be reflected in the UI, it must call `this.refreshProperties()` manually. Also, extends SeqVizParamModal.vue to present reasonable UI interfaces for a much larger variety of param types, as well as adds features such as hidable params, indentation, and so on. Resolves numberscope#55. Resolves numberscope#56. Resolves numberscope#60. Resolves numberscope#62. Resolves numberscope#82. (Although the graphic presentation for collapsible subgroups could be more polished.) Also takes care of numberscope#45 in all sequences and visualizers (there are still a few gratuitous console.logs in components, though). Lays groundwork for numberscope#68, in that the channel for communication between the UI and the sequence/visualizer is now very clear and as close to direct as possible (I don't think we want it any more direct; if it were, then in the midst of typing the visualizer could be left in an invalid state.)
gwhitney
added a commit
to gwhitney/frontscope
that referenced
this issue
Feb 15, 2022
To allow params to be of any type (rather than just `string | number | boolean` and to allow params to correspond to correctly-typed top-level properties, this commit creates a common base class Paramable for Sequences and Visualizers that handles communication of params with the UI. Basically, a class just derives from Paramable, defines a `params` property with objects describing the top-level properties that should be params, and defines a checkParameters() function that determines if a collection of candidate values (as `value` properties in the `params` object) are acceptable. If so, they are copied automatically into the implementing object's top-level properties. If the implementation changes any of these properties and wants the change to be reflected in the UI, it must call `this.refreshProperties()` manually. Also, extends SeqVizParamModal.vue to present reasonable UI interfaces for a much larger variety of param types, as well as adds features such as hidable params, indentation, and so on. Resolves numberscope#55. Resolves numberscope#56. Resolves numberscope#60. Resolves numberscope#62. Resolves numberscope#82. (Although the graphic presentation for collapsible subgroups could be more polished.) Also takes care of numberscope#45 in all sequences and visualizers (there are still a few gratuitous console.logs in components, though). Lays groundwork for numberscope#68, in that the channel for communication between the UI and the sequence/visualizer is now very clear and as close to direct as possible (I don't think we want it any more direct; if it were, then in the midst of typing the visualizer could be left in an invalid state.)
gwhitney
added a commit
to gwhitney/frontscope
that referenced
this issue
Feb 15, 2022
To allow params to be of any type (rather than just `string | number | boolean` and to allow params to correspond to correctly-typed top-level properties, this commit creates a common base class Paramable for Sequences and Visualizers that handles communication of params with the UI. Basically, a class just derives from Paramable, defines a `params` property with objects describing the top-level properties that should be params, and defines a checkParameters() function that determines if a collection of candidate values (as `value` properties in the `params` object) are acceptable. If so, they are copied automatically into the implementing object's top-level properties. If the implementation changes any of these properties and wants the change to be reflected in the UI, it must call `this.refreshProperties()` manually. Also, extends SeqVizParamModal.vue to present reasonable UI interfaces for a much larger variety of param types, as well as adds features such as hidable params, indentation, and so on. Resolves numberscope#55. Resolves numberscope#56. Resolves numberscope#60. Resolves numberscope#62. Resolves numberscope#82. (Although the graphic presentation for collapsible subgroups could be more polished.) Also takes care of numberscope#45 in all sequences and visualizers (there are still a few gratuitous console.logs in components, though). Lays groundwork for numberscope#68, in that the channel for communication between the UI and the sequence/visualizer is now very clear and as close to direct as possible (I don't think we want it any more direct; if it were, then in the midst of typing the visualizer could be left in an invalid state.)
liammulh
pushed a commit
that referenced
this issue
Mar 28, 2022
* refactor: first rough cut and direct parameter access (avoid typing step) * refactor: Overhaul param system To allow params to be of any type (rather than just `string | number | boolean` and to allow params to correspond to correctly-typed top-level properties, this commit creates a common base class Paramable for Sequences and Visualizers that handles communication of params with the UI. Basically, a class just derives from Paramable, defines a `params` property with objects describing the top-level properties that should be params, and defines a checkParameters() function that determines if a collection of candidate values (as `value` properties in the `params` object) are acceptable. If so, they are copied automatically into the implementing object's top-level properties. If the implementation changes any of these properties and wants the change to be reflected in the UI, it must call `this.refreshProperties()` manually. Also, extends SeqVizParamModal.vue to present reasonable UI interfaces for a much larger variety of param types, as well as adds features such as hidable params, indentation, and so on. Resolves #55. Resolves #56. Resolves #60. Resolves #62. Resolves #82. (Although the graphic presentation for collapsible subgroups could be more polished.) Also takes care of #45 in all sequences and visualizers (there are still a few gratuitous console.logs in components, though). Lays groundwork for #68, in that the channel for communication between the UI and the sequence/visualizer is now very clear and as close to direct as possible (I don't think we want it any more direct; if it were, then in the midst of typing the visualizer could be left in an invalid state.) * fix: changes as requested in review * docs: Clarify the meanings of ParamInterface properties * feat: Add 'integer' forceType for Paramable This special type for a parameter stores its result in a `number` but enforces in the UI that the value be an integer. This commit also adds this forceType to all Sequence and Visualizer parameters for which it is appropriate. That change noticeably reduces the amount of validation that needs to occur in the TypeScript code. * feat(params): When an input box "fixes" integer input, post a type reminder * refactor(param): Remove "indent" prop in favor of style for dependent params
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
In many Sequence implementations, there are params that have the 'required' flag set to true in their SequenceParam structure, but then the implementation checks to make sure the param was supplied in the validate() method. While one might appreciate the abundance of caution, this is actually a case of the same information being specified twice. The code should be enhanced to provide a guarantee that if the param is marked as required in the SequenceParam structure, then the sequence will not be constructed without that param; then individual implementations can stop checking that required parameters were in fact supplied (at least as long as they call their super.validate() method, at least, say).
The text was updated successfully, but these errors were encountered: