Client count config view #12422
Client count config view #12422
Conversation
- Switched to toggle button from checkbox and updated the design - Switched to ember octane - Update ember concurrency dependency
|
|
| modalTitle: computed('model.enabled', function() { | ||
| } | ||
|
|
||
| @computed('args.model.enabled') |
There was a problem hiding this comment.
I didn't know computed was a thing in octane?
There was a problem hiding this comment.
While the computed decorator is available in octane, I don't think its necessary in this case. I will remove it.
| <div class="control"> | ||
| <input | ||
| data-test-field | ||
| id={{attr.name}} |
There was a problem hiding this comment.
no chance there would be two of the same fields on the same page? e.g. two with the same attr.name?
There was a problem hiding this comment.
Nope, should not happen.
| data-test-field | ||
| id={{attr.name}} | ||
| disabled={{eq @model.enabled "Off"}} | ||
| readonly={{isReadOnly}} |
There was a problem hiding this comment.
is isReadOnly a local property (e.g. this.isReadOnly) or something passed in (e.g. @isReadOnly). With glimmer at least we should always see this or @ prefixing varaibles, unless their helpers. I think that's the standard, but correct me if I'm wrong.
There was a problem hiding this comment.
True. Good catch, in fact we don't need readonly attribute for these inputs.
| const fields = document.querySelectorAll('[data-test-field] input'); | ||
| assert.equal(fields.length, 3, 'renders 3 input fields'); | ||
| const fields = document.querySelectorAll('[data-test-field]'); | ||
| console.log(fields); |
* Client count config view - Switched to toggle button from checkbox and updated the design - Switched to ember octane - Update ember concurrency dependency * Fixed integration tests * Added changelog * Update switch label on toggle * Code cleanup * Fixed test
Design: https://www.figma.com/file/UEAB2ep3JpAZI0zZe05l48/Vault-Client-Counts?node-id=952%3A25956