refactor(major): ES6 classes, legacy build support, linting, only yield actions #28
Conversation
| @@ -0,0 +1,10 @@ | |||
| {{yield (hash | |||
| popper=(hash | |||
There was a problem hiding this comment.
@pzuraq you open to dropping the nested popper hash once able? I'm down to keep it in for now, but would love to see the yielded hash simplified in the future.
There was a problem hiding this comment.
yeah, definitely agree that it would be ideal to drop these. We can even drop them now if you're ok with the breaking changes (there are others like the class one) and only have them for the 1.11 version
|
|
||
| // ================== PRIVATE IMPLEMENTATION DETAILS ================== | ||
|
|
||
| _addPopper(target, element, options) { |
There was a problem hiding this comment.
as discussed on slack, would love to see the coupling torched
| let popperContainer; | ||
|
|
||
| if (renderInPlace) { | ||
| popperContainer = this.element.parentNode; |
There was a problem hiding this comment.
This borks fastboot since it doesn't have access to this.element. Previously we did this.
As long as we have a similar check for self.document and bail early, we should be good.
There was a problem hiding this comment.
If you want a way to sanity check, the dummy app is set up to use fastboot. In the long run, we should probably add some tests for this.
There was a problem hiding this comment.
ah, for sure, makes sense. I figured we could get a two-for-one by having renderInPlace to the document check for us as well, but I guess not. Will add an additional check.
kybishop
changed the title
| let popperContainer; | ||
|
|
||
| if (renderInPlace) { | ||
| popperContainer = this.element ? this.element.parentNode : ''; |
There was a problem hiding this comment.
This still goes 💥 in fastboot (all access to this.element is denied). self.document should do the trick.
There was a problem hiding this comment.
ah, for sure. Was thinking it would look more consistent, but yeah that's probably safer
kybishop
changed the title
kybishop
changed the title
…re actions Changes everything to ES6 classes, adds legacy build support all the way back to Ember 1.11, adds stricter linting rules, yields only actions
Fixes #20