refactor(argument): Update to use @argument #479
Conversation
| // We have to alias because the class name changes per base cell type | ||
| @className('', 'et-td') | ||
| @readOnly @alias('isFixed') _isFixed; | ||
|
|
There was a problem hiding this comment.
Minor: remove this blank line
There was a problem hiding this comment.
Think you could handle this? Don’t have my computer right now 😅
|
|
||
| // We have to alias because the class name changes per base cell type | ||
| @className('', 'et-td') | ||
| @readOnly @alias('isFixed') _isFixed; |
There was a problem hiding this comment.
I am not quite sure how the _isFixed works here? It's not read anywhere. Can you explain more here?
There was a problem hiding this comment.
@className binds the property to a class name, and we provide those class names and the arguments. We can do it directly to isFixed because of problems with how it works in ember-decorators, so we need to make an alias.
There was a problem hiding this comment.
Going to open up a bug report in ember-decorators, but this would be a breaking change so we’ll need to hold off for some time.
|
|
||
| export default class CellProxy extends EmberObject { | ||
| @property column = null; | ||
| column = null; |
There was a problem hiding this comment.
Do you want to put @argument decorator for this? This is a required field for cell proxy
There was a problem hiding this comment.
@argument is meant for components, its dynamics don’t work really well here yet. We can update in the future, but it’s tricky so I kept it simple.
There was a problem hiding this comment.
We could just use @type here potentially.
|
Looks good overall. Just a few comment & question |
|
hmm, I got |
|
Do you mean in Iverson? If so, that means we have a lack of test coverage here. |
|
No, even with this repo. The demo app fails to load |
pzuraq
force-pushed
the
pzuraq/refactor/upgrade-decorators
branch
from
425a738 to
47b2e5c
Compare
pzuraq
force-pushed
the
pzuraq/refactor/upgrade-decorators
branch
from
52a83d2 to
b8e5c87
Compare
Refactors to use @argument, @type, and other new decorators. Cleans up some unused code as well, and in general straightens things out (declaring all arguments, removing unused ones, etc.)
cc @twokul @cyril-sf @billy-addepar