UI/tools partial #11672
UI/tools partial #11672
Conversation
| @@ -135,14 +135,11 @@ export default Component.extend(DEFAULTS, { | |||
| this.reset(); | |||
| }, | |||
|
|
|||
| updateTtl(evt) { | |||
| const ttl = evt.enabled ? `${evt.seconds}s` : '30m'; | |||
There was a problem hiding this comment.
the ttl is determined in the tool-wrap component.
| set(this, 'wrapTTL', ttl); | ||
| }, | ||
|
|
||
| codemirrorUpdated(val, codemirror) { | ||
| codemirror.performLint(); |
There was a problem hiding this comment.
codemirror things need to happen inside the component where codemirror is being used, thus the property codemirror is used in the tool-wrap component.
| export default class HashTool extends Component { | ||
| @action | ||
| onClear() { | ||
| this.args.onClear(); |
There was a problem hiding this comment.
for all new components the onClear() action is passed through to the child as a property and then called again via an action.
| const hasErrors = codemirror.state.lint.marked.length > 0; | ||
| this.data = val; | ||
| this.buttonDisabled = hasErrors; | ||
| this.args.codemirrorUpdated(val, hasErrors); |
There was a problem hiding this comment.
now we call the parent's codemirrorUpdated function and instead of passing in val, codemirror we pass in val, hasErrors. Again, the codemirror property is only available inside the instance of the component it is used, it cannot be passed through as a property.
| @onClear={{action "onClear"}} | ||
| @errors={{errors}} | ||
| /> | ||
| {{else if (eq selectedAction 'wrap')}} |
There was a problem hiding this comment.
notice these are all else ifs, there is no fallback component. Technically there doesn't need to be, but curious if folks take issue with this.
There was a problem hiding this comment.
I think this is the right call, unless we want to add an empty state (which would most likely be viewed by a developer updating this space)
There was a problem hiding this comment.
Good idea with the empty state. It feels strange to have no else so I'll go ahead and add it.
ui/app/components/tool-hash.js
Outdated
| import Component from '@glimmer/component'; | ||
| import { action } from '@ember/object'; | ||
|
|
||
| export default class HashTool extends Component { |
There was a problem hiding this comment.
Might be nice to add JSDocs here and include onClear as a required attr, as well as anything else used on the template
ui/app/components/tool-wrap.js
Outdated
| @action | ||
| codemirrorUpdated(val, codemirror) { | ||
| codemirror.performLint(); | ||
| const hasErrors = codemirror.state.lint.marked.length > 0; |
There was a problem hiding this comment.
I wonder if we should use optional chaining here in case marked or anything upstream comes back undefined?
| @@ -1,3 +1,59 @@ | |||
| <form onsubmit={{action "doSubmit"}}> | |||
| {{partial (concat "partials/tools/" selectedAction)}} | |||
| {{#if (eq selectedAction 'hash')}} | |||
| <ToolHash | |||
| } | ||
| @action | ||
| codemirrorUpdated(val, codemirror) { | ||
| codemirror.performLint(); |
There was a problem hiding this comment.
Can codemirror be undefined or null?
There was a problem hiding this comment.
This is also how we handle it in other parts of the app.
| set(this, 'wrapTTL', ttl); | ||
| }, | ||
|
|
||
| codemirrorUpdated(val, codemirror) { | ||
| codemirror.performLint(); |
There was a problem hiding this comment.
Same, can codemirror be undefined or null?
There was a problem hiding this comment.
nope, it's always an object that's returned.
| * @param sum=null {String} - property passed from parent to child and then passed back up to parent | ||
| * @param algorithm {String} - property returned from parent. | ||
| * @param format {String} - property returned from parent. | ||
| * @param error=null {Object} - errors passed from parent as default then from child back to parent. |
There was a problem hiding this comment.
Nice job with js doc, I should add it as well 😄
In an effort to get ahead of future Ember upgrade depreciations we are trying to replace our use of partials with components. This tackles the Tools area.
In the end there is slightly more code, but it is much easier to read.