-
Notifications
You must be signed in to change notification settings - Fork 6k
[web] JSConfig: Add multiViewEnabled value. #47939
Conversation
harryterkelsen
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.
LGTM
mdebbar
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.
Ship it!
| /// Multi-view mode allows apps to: | ||
| /// | ||
| /// * Start without a `hostElement`. | ||
| /// * Add/remove views (`hostElements`) from JS while the application is running. |
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.
🎉
|
Did I really break a pointer test with this? Odd. I'm going to rebase and see what CI thinks about my branch :/ |
|
|
||
| external DomElement? get hostElement; | ||
|
|
||
| @JS('multiViewEnabled') |
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.
We can give any "external" (JS) name we want to this property, like "multiViewEnabledExperimental" or something scary like that. Do we need that, or do we just call it "multiViewEnabled"? @mdebbar?
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 it's fine to keep the name multiViewEnabled to avoid a little bit of churn down the line. Anyone who makes use of this flag ought to know that this is experimental and could break at any time.
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 I agree with this!
|
I'll land this after @eyebrowsoffire "flutter.js in tests" PR lands, to not cause any conflicts on this end. |
8395279 to
1a0aae0
Compare
|
Rebased and ready to land, applying |
…138457) flutter/engine@d7ca057...bc5bbd3 2023-11-15 [email protected] [web] JSConfig: Add multiViewEnabled value. (flutter/engine#47939) 2023-11-15 [email protected] [Impeller] Simplify convex tessellation (flutter/engine#47957) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
This change:
multiViewEnabled(defaults tofalse).canvasKitMaximumSurfacesvalue.Part of: flutter/flutter#137377
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.