Adding new design property.
#51
Conversation
|
Update the README as well? |
|
Yeah - this should be documented. Will add. |
| @@ -100,7 +122,15 @@ export default Ember.Component.extend({ | |||
| * ['small', 'medium', 'large', 'extra-large'] | |||
| * @type {String} | |||
| */ | |||
| size: 'medium', | |||
| size: '', | |||
There was a problem hiding this comment.
It looks like this constitutes a breaking API change and therefore warrants a #MAJOR# version-bump
There was a problem hiding this comment.
I just realized this when I updated the readme. Will update to #major#.
There was a problem hiding this comment.
Is it really necessary to remove these defaults for size and priority ?
There was a problem hiding this comment.
When design is present, we don't want priority and size to be specified. If they are present, we will ignore them and print the warning. Since they have a value by default, we would always end up with the warning even if the application did not specify it. So it is a bit cleaner without the default values.
There was a problem hiding this comment.
Got it. Well in that case, I guess we need the #MAJOR# bump then because there is existing code which probably relies on the defaults and that code will break with this release.
There was a problem hiding this comment.
Agreed - I updated that.
There was a problem hiding this comment.
Could potentially have addressed this with internal _priority set to default and external priority setting that value/being checked against the assertion, but this is already merged...so ;)
In the future (once this is in core) - I want multiple approvals on any #major# changes, maybe we can do that will pullapprove
|
👍 |
Adding new `design` property.
| autofocus=true | ||
| on-click=(action 'click') | ||
| design='tab' | ||
| text='Design Tab' |
major