Conversation
nummi
commented
Aug 28, 2019
nummi
commented
- @
- this
- action
- on
- @ - this - action - on
RobbieTheWagner
left a comment
RobbieTheWagner
left a comment
There was a problem hiding this comment.
Just a few questions, but overall looking good!
|
|
||
| export default Component.extend({ | ||
| // passed as an attribute to the component | ||
| currentRoute: null, |
There was a problem hiding this comment.
Do we no longer want these defaults to null?
There was a problem hiding this comment.
I removed these because sooner or later this will become a Glimmer component and currentRoute will live in the args hash.
There was a problem hiding this comment.
I'm happy to put it back for now.
| id="options-hideRoutes" | ||
| }} | ||
| <Input | ||
| @id="options-hideRoutes" |
There was a problem hiding this comment.
Should this be id instead of @id? Assuming id is meant to be set on the element. Not sure if the input helper is a special case or not. Have you checked in the DOM to ensure the id is there and the input is working as expected?
There was a problem hiding this comment.
I'm just doing what the docs tell me :)
https://guides.emberjs.com/v3.12.0/templates/input-helpers/#toc_text-fields
There was a problem hiding this comment.
Fair enough. I guess the input helper is a special case
| id="options-hideSubstates" | ||
| }} | ||
| <Input | ||
| @id="options-hideSubstates" |
|
|
||
| <div class="toolbar__search js-filter-views"> | ||
| {{ui/toolbar-search-field value=searchValue}} | ||
| {{ui/toolbar-search-field value=this.searchValue}} |
There was a problem hiding this comment.
Should we make this <Ui::Toolbar etc?
There was a problem hiding this comment.
I was going to take another pass and convert all the Ui components at one time.
* Remove unused component template * Modernize Route tab - @ - this - action - on
2 participants
