-
Notifications
You must be signed in to change notification settings - Fork 44
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
Pass in style attribute to style attachment #136
Pass in style attribute to style attachment #136
Conversation
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 @jrjohnson! This is looking great, just a few minor comments.
addon/components/attach-popover.js
Outdated
@@ -39,6 +40,7 @@ export default Component.extend({ | |||
} | |||
}), | |||
class: DEFAULTS.class, | |||
style: DEFAULTS.style, |
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.
Bikeshed: Let's keep these alphabetized by moving this to the bottom of this property list.
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.
No problem, done.
addon/components/attach-popover.js
Outdated
_style: computed('style', '_transitionDuration', function () { | ||
const style = this.get('style'); | ||
const transitionDuration = this.get('_transitionDuration'); | ||
assert( |
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.
Doesn't ember normally warn when we pass a style that wasn't marked with htmlSafe
? If so, I think we should follow suit over asserting.
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.
Good call. I thought it was an error, but it is, in fact, just a warning. Fixed
addon/defaults.js
Outdated
@@ -2,6 +2,7 @@ export default { | |||
animation: 'fill', | |||
arrow: false, | |||
class: null, | |||
style: null, |
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.
Let's alphabetize this, too.
d222779
to
e403ca9
Compare
Allows passing in a safe string as a style which will be applied to the tooltip or attachment element.
e403ca9
to
f78f427
Compare
Allows passing in a safe string as a style which will be applied to the
tooltip or attachment element.
Fixes #135