-
Notifications
You must be signed in to change notification settings - Fork 74
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
Remove widgets individually in the protocol #2503
Conversation
ea052a6
to
33affc0
Compare
This is required to efficiently implement movableContentOf in a future change.
33affc0
to
36abbaa
Compare
|
||
@Suppress("DEPRECATION") // Testing for old behavior. | ||
@Test | ||
fun removeRangeForOldVersions() = runTest { |
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.
nice
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'm actually thinking this behavior switching is pointless now. I broke this change out of the large one, but it's always valid to send individual removes to any host. So we always can do it and only worry about whether or not it's valid to send detach=true
or not.
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.
Just going to leave it for now, land the rest which are kinda stacked on it, and then maybe delete.
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.
Yeah I got rid of this in #2509.
@@ -208,7 +214,13 @@ internal class FastGuestProtocolAdapter( | |||
val tag = tag | |||
val index = index | |||
val count = count | |||
changes.push(js("""["remove",{"id":id,"tag":tag,"index":index,"count":count}]""")) | |||
if (hostSupportsRemoveDetachAndNoCount) { |
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 assume in follow-up we'll change the caller to use a different function?
This is required to efficiently implement
movableContentOf
in a future change.See #1902
CHANGELOG.md
's "Unreleased" section has been updated, if applicable.