-
Notifications
You must be signed in to change notification settings - Fork 4
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
add MutationObserver
#72
Conversation
|
||
def disconnect: F[Unit] | ||
|
||
def takeRecords: F[List[dom.MutationRecord]] |
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.
Shoot, I wasn't paying enough attention last time. Even though dom.MutationRecord
and dom.ResizeObserverEntry
are immutable objects which we don't need to wrap, they are referencing many things which we probably do want to wrap. Gah.
|
||
import org.scalajs.dom | ||
|
||
abstract class ResizeObserverEntry[F[_]] private[dom] { |
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.
Perhaps something like this?
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'll take a more detailed look later but this is exactly what I was imagining!! Thank you so much, really appreciate it :)
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.
Cool, I can add something similar for dom.MutationRecord
tomorrow.
|
||
abstract class MutationObserver[F[_]] private[dom] { | ||
|
||
def observe(target: Node[F], options: dom.MutationObserverInit): F[Unit] |
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.
Shall we also provide a wrapper for dom.MutationObserverInit
?
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.
And dom.ResizeObserverOptions
?
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 question :) ok, so usual disclaimer about arbitrariness ...
I've been trying to just reuse these *Init
and *Options
types whenever possible, so that we don't have to wrap everything. They are mutable, which is not great, but I think their usual usage pattern is as builders local to the method call, rather than passing them all over the place. So probably fine. And all their member types are simple stuff (strings, booleans, enums). So IMO the trade-off is not worth it: if we wrapped it, it would be more annoying for us to maintain, and more annoying for the user to use, so lose-lose.
def `type`: String | ||
|
||
def target: Node[F] | ||
|
||
def addedNodes: List[Node[F]] | ||
|
||
def removedNodes: List[Node[F]] | ||
|
||
def previousSibling: Node[F] | ||
|
||
def nextSibling: Node[F] | ||
|
||
def attributeName: String | ||
|
||
def attributeNamespace: String | ||
|
||
def oldValue: String |
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 looks like some of these can be null, I guess we should wrap them in Option
.
https://developer.mozilla.org/en-US/docs/Web/API/MutationRecord
override def target: Node[F] = record.target.asInstanceOf[Node[F]] | ||
|
||
override def addedNodes: List[Node[F]] = | ||
record.addedNodes.toList.map(_.asInstanceOf[Node[F]]) |
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.
Performance hack 😛
record.addedNodes.toList.map(_.asInstanceOf[Node[F]]) | |
record.addedNodes.toList.asInstanceOf[List[Node[F]]] |
|
||
abstract class MutationObserver[F[_]] private[dom] { | ||
|
||
def observe(target: Node[F], options: dom.MutationObserverInit): F[Unit] |
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 question :) ok, so usual disclaimer about arbitrariness ...
I've been trying to just reuse these *Init
and *Options
types whenever possible, so that we don't have to wrap everything. They are mutable, which is not great, but I think their usual usage pattern is as builders local to the method call, rather than passing them all over the place. So probably fine. And all their member types are simple stuff (strings, booleans, enums). So IMO the trade-off is not worth it: if we wrapped it, it would be more annoying for us to maintain, and more annoying for the user to use, so lose-lose.
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 for all your work on this!
|
||
abstract class MutationRecord[F[_]] private[dom] { | ||
|
||
def `type`: String |
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 think in scala-js-dom we should make this an enum (which is a compatible change) but it might become an incompatible change for us here. Too bad, we'll deal with it when it happens 🙃
No description provided.