Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[web] Support platform view creation params #42255

Merged
merged 5 commits into from
Jun 2, 2023

Conversation

mdebbar
Copy link
Contributor

@mdebbar mdebbar commented May 23, 2023

  1. Wire platform views' creation params in the engine.
  2. Move acceptable factory signatures to dart:ui_web.
  3. Don't put any JS types in dart:ui_web's interface.
    • Use type casting to check at runtime instead.
  4. Tests.

Part of flutter/flutter#127030

@mdebbar mdebbar requested review from ditman and harryterkelsen May 23, 2023 18:48
@flutter-dashboard flutter-dashboard bot added the platform-web Code specifically for the web engine label May 23, 2023
Copy link
Member

@ditman ditman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's go creation params!

if (factoryFunction is ParameterizedPlatformViewFactory) {
content = factoryFunction(viewId, params: params);
if (factoryFunction is ui_web.ParameterizedPlatformViewFactory) {
content = factoryFunction(viewId, params: params) as DomElement;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this assert(content is DomElement, 'Cool error message'), rather than just the cast? That way we may be able to provide a better error message than the default one coming from Dart?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good idea!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I don't think DomElement is a real type at runtime, so I'm not sure what content is DomElement is going to check. cc @joshualitt any ideas?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to submit this as is. I can do a follow up PR if you still feel strongly about it.

Comment on lines +154 to +167
expect(factoryCalls[1].viewId, 222);
expect(factoryCalls[1].params, <dynamic, dynamic>{'foo': 'bar'});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need an extra case where the param passed is not a map, but a string or an int, or some other simple Object?

@mdebbar mdebbar force-pushed the creation_params branch from 777d15f to c325e82 Compare June 2, 2023 20:06
@mdebbar mdebbar added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 2, 2023
@auto-submit auto-submit bot merged commit ba0dedd into flutter:main Jun 2, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 2, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 2, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 3, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 3, 2023
goderbauer pushed a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 3, 2023
goderbauer pushed a commit to flutter/flutter that referenced this pull request Jun 3, 2023
…128158)

flutter/engine@8769e9c...5429372

2023-06-03 [email protected] [Impeller] Fix 1-d grid computation
for compute (flutter/engine#42516)
2023-06-02 [email protected] Roll Fuchsia Linux SDK from
PuYA-6NVHeHPlkCdk... to VtLnfLmVda1_h1AtM... (flutter/engine#42529)
2023-06-02 [email protected] [macOS] Top-left origin for PlatformView
container (flutter/engine#42523)
2023-06-02 [email protected] Manual roll Dart SDK from
9d8df2a5210b to d198f84f5e4e (1 revision) (flutter/engine#42527)
2023-06-02 [email protected] Revert "Reland "add non-rendering operation
culling to DisplayListBuilder" (#41463)" (flutter/engine#42525)
2023-06-02 [email protected] Move benchmarks no upload to staging.
(flutter/engine#42524)
2023-06-02 [email protected] [web] Support platform view creation
params (flutter/engine#42255)
2023-06-02 [email protected] MultiView changes for dart:ui
(flutter/engine#42493)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from PuYA-6NVHeHP to VtLnfLmVda1_

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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jun 15, 2023
This concludes step 1 of the `HtmlElementView` improvements. It's now possible to pass creation params to platform view factories directly from `HtmlElementView`.

Here's a sample app using a single factory to render platform views in different colors:

<details>
  <summary>Code sample</summary>
  
  ```dart
import 'dart:js_interop';
import 'dart:ui_web' as ui_web;
import 'package:flutter/material.dart';
import 'package:web/web.dart' as web;

void main() {
  runApp(MyApp());
}

class MyApp extends StatelessWidget {
  @OverRide
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Platform View Demo',
      home: Scaffold(
        appBar: AppBar(
          title: Text('Platform View Demo'),
        ),
        body: Center(
          child: Column(
            mainAxisAlignment: MainAxisAlignment.spaceEvenly,
            children: [
              BoxWrapper('red'),
              BoxWrapper(null),
              BoxWrapper('blue'),
            ],
          ),
        ),
      ),
    );
  }
}

bool isRegistered = false;

class BoxWrapper extends StatelessWidget {
  const BoxWrapper(this.cssColor);

  final String? cssColor;

  void register() {
    if (isRegistered) return;
    isRegistered = true;

    ui_web.platformViewRegistry.registerViewFactory('my-platform-view', (
      id, {
      Object? params,
    }) {
      params as String?;

      final element = web.document.createElement('div'.toJS) as web.HTMLElement;
      element.textContent = 'Platform View'.toJS;
      element.style
        ..lineHeight = '100px'.toJS
        ..fontSize = '24px'.toJS
        ..backgroundColor = (params ?? 'pink').toJS
        ..textAlign = 'center'.toJS;
      return element;
    });
  }

  @OverRide
  Widget build(BuildContext context) {
    register();
    return SizedBox(
      width: 200,
      height: 100,
      child: Card(
        child: HtmlElementView(
          viewType: 'my-platform-view',
          creationParams: cssColor,
        ),
      ),
    );
  }
}
  ```
</details>

![image](https://github.com/flutter/flutter/assets/1278212/4b62ed8b-2314-49d6-9b4a-5da849bf2a48)

Depends on flutter/engine#42255

Part of #127030
@mdebbar mdebbar deleted the creation_params branch June 22, 2023 21:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App platform-web Code specifically for the web engine
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants