-
Notifications
You must be signed in to change notification settings - Fork 13
Replaced ember-resize-observer-service with ember-primitives/resize-observer #277
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
Replaced ember-resize-observer-service with ember-primitives/resize-observer #277
Conversation
a286729 to
dfab8b3
Compare
|
|
||
| registerDestructor(this, () => { | ||
| this.resizeObserver.unobserve(this._element, this.onResize); | ||
| if (this._element) { |
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.
To confirm, did you add this if-statement just for TypeScript (Argument of type 'Element | undefined' is not assignable to parameter of type 'Element'.), or was it also because the modifier had otherwise resulted in a runtime error in docs-app?
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.
It was in response to that ts message, yes
ijlee2
left a comment
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.
I reduced the number of commits for posterity. Thanks for keeping this addon's dependencies up-to-date.
|
I think export default class ContainerQuery<T extends IndexSignatureParameter> extends Modifier<ContainerQuerySignature<T>> {
#private;
private _dataAttributes;
private _element?;
private _named;
// ...
}Should this be considered a bug in private _resizeObserver = resizeObserver(this); |
I don't think so, #privateFields aren't meant to be knowable from the outside, so i think it's fine |
|
Sounds good, thanks again! |
With all the v1 addons gone now, ember-container-query is compatible with vite-native (no compat) applications
Pending: