-
Notifications
You must be signed in to change notification settings - Fork 6k
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
[web] Add dynamic view sizing (v2) #50271
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
Hey @harryterkelsen, @mdebbar, I came up with a much simpler solution to the previous one as I was updating this. PTAQL! |
@@ -156,6 +168,17 @@ base class EngineFlutterView implements ui.FlutterView { | |||
return _physicalSize ??= _computePhysicalSize(); | |||
} | |||
|
|||
void resize(ui.Size newPhysicalSize) { | |||
// The browser wants logical sizes! |
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 should really have separate PhysicalSize
and LogicalSize
classes so we don't confuse the two (with methods to convert from one to the other e.g. PhysicalSize.toLogicalSize(dpr)
and LogicalSize.toPhysicalSize(dpr)
). Maybe when extension type
is a thing we can give this a try!
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, Size is very generic, but valid most of the time. We really only care about this in the interfaces between systems:
Framework (logical, most of the time) <-> Engine (physical, most of the time) <-> Browser (logical, always?)
This sounds like an error waiting to happen (well, my earlier HTML renderer PR is an example of that exact error, I guess! :P)
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 isn't the same as having two separate types, but I've started being very careful documenting the type of size required depending on where in the code we are: if values come from JS/CSS/the browser in general -> logical size; if we're passing things to the framework -> physical size. Also attempted to reflect that in the variable/parameter names.
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, ship it!
1f8fd7f
to
ae22b2c
Compare
Added a bunch more tests and extended the comments in the Constraints calculation, to reason why things do what they do. PTAL @mdebbar, I think this is ready to land. |
b803605
to
9e7d3d6
Compare
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.
Still LGTM!
(It's removed in dart:ui)
9e7d3d6
to
e5cb79c
Compare
All right, I'll |
This comment was marked as resolved.
This comment was marked as resolved.
Mmmmkay... |
I ran the failing test locally, and it seems to pass:
(Maybe related to this revert in the framework?) I'll set |
### Changes * Introduces a new `viewConstraints` JS configuration parameter to configure max/min width/height constraints for a view. Those can have the following values: * An integer `>= 0`: max/min size in pixels * `Infinity` (or `Number.POSITIVE_INFINITY`): (only for max values) -> **unconstrained**. * When any value is not set, it defaults to "tight to the current size". * See [Understanding constraints](https://docs.flutter.dev/ui/layout/constraints). * Computes the correct `physicalConstraints` of a view off of its `physicalSize` and its `viewConstraints` for the framework to use during layout. * When no constraints are passed, the current behavior is preserved: the default constraints are "tight" to the `physicalSize`. * Resizes the current view DOM when requested by the framework and updates its internal physicalSize, then continues with the render procedure. ### Example This is how we can configure a view to "take as much vertical space as needed": ```js flutterApp.addView({ viewConstraints: { minHeight: 0, maxHeight: Infinity, }, hostElement: ..., }); ``` ### TODO * Needs actual unit tests ### Issues * Fixes flutter/flutter#137444 * Closes flutter#48541 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
2024-02-15 [email protected] [Android] Minor refactor: Remove redundant methods. (flutter/engine#50647) 2024-02-15 [email protected] [Impeller] consolidate transforms in PositionUVWriter (flutter/engine#50635) 2024-02-15 [email protected] [web] Add dynamic view sizing (v2) (flutter/engine#50271) 2024-02-14 [email protected] Add useful default options to scenario_app/bin/android_integration_tests.dart (flutter/engine#50667) 2024-02-14 [email protected] Fix github md "Note" and "Tip" blocks in Android shell README (flutter/engine#50664) 2024-02-14 [email protected] [Fuchsia] Run run_with_dart_aot test on fuchsia_profile_x64 (flutter/engine#50613) 2024-02-14 [email protected] Make all Android scenario_app activities full-screen, even on older Android versions. (flutter/engine#50666) 2024-02-14 [email protected] Roll Skia from eae42ea9f7bc to 2d5cf67614d0 (6 revisions) (flutter/engine#50660) 2024-02-14 [email protected] Add a comment explaining the lifecycle of tls_command_pool_map. (flutter/engine#50623) 2024-02-14 [email protected] [web] Increase tolerance for golden diffs on Safari (flutter/engine#50655)
2024-02-15 [email protected] [Android] Minor refactor: Remove redundant methods. (flutter/engine#50647) 2024-02-15 [email protected] [Impeller] consolidate transforms in PositionUVWriter (flutter/engine#50635) 2024-02-15 [email protected] [web] Add dynamic view sizing (v2) (flutter/engine#50271) 2024-02-14 [email protected] Add useful default options to scenario_app/bin/android_integration_tests.dart (flutter/engine#50667) 2024-02-14 [email protected] Fix github md "Note" and "Tip" blocks in Android shell README (flutter/engine#50664) 2024-02-14 [email protected] [Fuchsia] Run run_with_dart_aot test on fuchsia_profile_x64 (flutter/engine#50613) 2024-02-14 [email protected] Make all Android scenario_app activities full-screen, even on older Android versions. (flutter/engine#50666) 2024-02-14 [email protected] Roll Skia from eae42ea9f7bc to 2d5cf67614d0 (6 revisions) (flutter/engine#50660) 2024-02-14 [email protected] Add a comment explaining the lifecycle of tls_command_pool_map. (flutter/engine#50623) 2024-02-14 [email protected] [web] Increase tolerance for golden diffs on Safari (flutter/engine#50655)
Changes
viewConstraints
JS configuration parameter to configure max/min width/height constraints for a view. Those can have the following values:>= 0
: max/min size in pixelsInfinity
(orNumber.POSITIVE_INFINITY
): (only for max values) -> unconstrained.physicalConstraints
of a view off of itsphysicalSize
and itsviewConstraints
for the framework to use during layout.physicalSize
.Example
This is how we can configure a view to "take as much vertical space as needed":
TODO
Issues
Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.