Conversation
…rametrs replace by default in the rendered traslation
chrisknoll
left a comment
There was a problem hiding this comment.
When the view initially presents, the options text input fields are read only. However, you can click 'replace values' (which will replace the defaults in the SQL output, and then the fields become enabled. I think these text fields should always be enabled (ie: never read-only).
There was a problem hiding this comment.
Let's make some changes to behave more like a knockout application where we depend on observables instead of managing our own subscriptions:
- Replace ko.compuated() with ko.pureComputed when the observable is just a computed alue (ie: the sqlParams).
- Remove subscriptions and instead just store user-entered fields as observables.
- sqlParamsList should return a key-value where the key is the param name, and the value is an observable(). This way, changing the value of the input will automatically trigger the refresh of the SQL.
- sqlText should be a pureCompuated()) calculated by replacing the sqlText() with the specified paramater values in sqlParamsList (if checkbox is checked) and then render the html with highlightJS().
The flow of the component should be:
- The root 'sql' comes is as a component param and is observable (which will update when cohort definition changes or a new cohort definition is loaded)
- sqlParams (pureCompted returning an array) depends on
sql, which will yield a name-value pairs of paramater names and observables() which store the user input. This pureCompted will be used in theforeach: sqlParams. If the sql canges in some way, this will refresh these params. and re-render the foreach elements. - The input fields can be bound to the
value:databinding so that they don't update the observables until the textbox is unfocused (via tabbing off the field). - sqlText() will be a pureComputed() which will generate the HTML to render in the
preelement. This can be bound to a pureCompuated which will do the appropraite find/replace and SQL highlighting from sql. This means that if sql, sqlParams, or the observables inside sqlParams change (via the textboxes), this computed will refresh and the<pre>element will be updated.
This should eliminate the need to maintain your own subscriptions, and handle any 'input' events in the event handler.
In addition, is the spread ... operator necessary, can we not just return a 'Set' here:
sqlParamsList(templateSql) {
const regexp = /@[-\w]+/g;
const params = templateSql.match(regexp);
const paramsList = new Set(params);
return [...paramsList];
}
|
@chrisknoll Hello Chris! Thank you for your comment. I changed the logic of component after that. I tried to realize the full flowm but i didn't. |
Resolves #2775