1.0: paper-progress-linear #277
Conversation
|
Could you please add more test cases like the ones here: Or maybe inspired by paper-progress-circular ones. I feel this component is still poorly tested. Let's ignore accessibility for now. We'll deal with that in a separate issue. |
| @@ -1,5 +1,7 @@ | |||
| import Ember from 'ember'; | |||
| import ColorMixin from 'ember-paper/mixins/color-mixin'; | |||
| const {inject, computed, Component, isPresent} = Ember; | |||
There was a problem hiding this comment.
to conform with other stlyes, please add spaces around declarations.
const { inject, computed, Component, isPresent } = Ember;
Sorry for this. We need to add decent jscs support in the project.
|
Also, we currently have a problem with component attributes. We have both I'll discuss this with ember folks at slack. Thank you so much for this, @peec. |
|
|
||
| init() { | ||
| this._super(...arguments); | ||
| this.setupTransforms(); | ||
| }, | ||
|
|
||
| mode: Ember.computed('value', function() { | ||
| mode: computed('value', function() { | ||
| var value = this.get('value'); | ||
| var bufferValue = this.get('buffer-value'); |
There was a problem hiding this comment.
We're now using let instead of var, expect for constants.
| assert.equal(this.$('md-progress-linear').attr('md-mode'), 'buffer'); | ||
| }); | ||
|
|
||
| test('should auto-set the md-mode to "buffer" if a value and bufferValue is specified', function(assert) { |
| }), | ||
|
|
||
| bar2Style: Ember.computed('clampedValue', function() { | ||
| bar2Style: computed('clampedValue', function() { | ||
|
|
||
| if (this.get('mode') === 'query') { |
There was a problem hiding this comment.
test against MODE_QUERY constant.
| clampedValue: Ember.computed('value', function() { | ||
|
|
||
| var value = this.get('value'); | ||
| clampedValue: computed('value', function() { |
There was a problem hiding this comment.
Here is a much cooler way to clamp values: https://github.com/miguelcobain/ember-paper/blob/wip/v1.0.0/addon%2Fcomponents%2Fpaper-progress-circular.js#L29-L32
I'll make this a computed macro in the future.
Can you change both cps here to clamp using similar code?
…t its purpose is to be used in the template.
|
Perfect. Let's just wait for some input on |
|
@miguelcobain Yes I agree, best to follow the ember-way on attribute naming, if they have suggestions |
|
@miguelcobain I will merge this as it holds up my wip/v1.0.0 branch (should have branched it to wip/v1.0.0/paper-progress-linear instead). And we can discuss further in issue of wip1.0.0 i guess ? |
Update code-style, docs and unit -> integration test.