-
Notifications
You must be signed in to change notification settings - Fork 286
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tidy up Promise tab #1038
Tidy up Promise tab #1038
Conversation
- @, this, </>, action, etc.
@@ -23,26 +23,26 @@ | |||
<Ui::ToolbarDivider /> | |||
|
|||
<button | |||
class="{{if noFilter "active"}} toolbar__radio toolbar__radio--first js-filter" | |||
{{action "setFilter" "all"}} | |||
class="{{if (eq this.filter "all") "active"}} toolbar__radio toolbar__radio--first js-filter" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥
class=(concat "list__cell_main " expandedClass) | ||
style=labelStyle | ||
on-click=(action toggleExpand model) | ||
{{#@list.cell |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this @
on purpose? I suppose it is valid, but looks odd having #@
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks terrible :-P. But list
is passed in that way, right? Maybe we should get a consult from an expert?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does angle bracket syntax work here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! That looks much better 😃
app/templates/promise-tree.hbs
Outdated
@model={{content}} | ||
@filter={{this.filter}} | ||
@effectiveSearch={{this.effectiveSearch}} | ||
@toggleExpand={{fn this.toggleExpand content}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious why only two of the actions needed content
passed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like to keep the components as dumb as possible but maybe consistency should win here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If only two need it, that's fine. I just wanted some clarity.
sendValueToConsole=(action "sendValueToConsole") | ||
list=list | ||
}} | ||
<PromiseItem |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fancy!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. 😆
code climate 🤷♂ |
* Tidy up Promise tab - @, this, </>, action, etc. * Consistent action passing to PromiseItem * Angle bracket for promise table cells
@, this, </>, action
,etc.