Skip to content

[web] New HtmlElementView.fromTagName constructor #130513

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 12 commits into from
Aug 7, 2023
Merged

[web] New HtmlElementView.fromTagName constructor #130513

merged 12 commits into from
Aug 7, 2023

Conversation

mdebbar
Copy link
Contributor

@mdebbar mdebbar commented Jul 13, 2023 •
edited
Loading

This wraps up the platform view improvements discussed in #127030.

  • Splits HtmlElementView into 2 files that are conditionally imported.
  • The non-web version can be instantiated but it throws if it ends up being built in a widget tree.
  • Out-of-the-box view factories that create visible & invisible DOM elements given a tagName parameter.
  • New HtmlElementView.fromTagName() constructor that uses the default factories to create DOM elements.
  • Tests covering the new API.

Depends on flutter/engine#43828

Fixes #127030

@mdebbar mdebbar requested a review from ditman July 13, 2023 19:16
@github-actions github-actions bot added the framework flutter/packages/flutter repository. See also f: labels. label Jul 13, 2023
ditman
ditman reviewed Jul 14, 2023
View reviewed changes
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.

Small comment: would defining the default factories in the engine simplify the framework code?

@mdebbar mdebbar mentioned this pull request Jul 19, 2023
Merged
ditman
ditman reviewed Jul 21, 2023
View reviewed changes
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.

I think this one is ready to go. Only one comment about attempting to make the new extension private so it doesn't show in docs.flutter.dev.

@mdebbar mdebbar changed the title [web] New HtmlElementView.domElement constructor [web] New HtmlElementView.fromTagName constructor Jul 25, 2023
ditman

This comment was marked as outdated.

Sign in to view

auto-submit bot pushed a commit to flutter/engine that referenced this pull request Jul 26, 2023
@mdebbar
[web] Provide convenient default factories for platform views (#43828)
6f1c2df 
Convenient default factories for creating DOM element from a given tag name.

Required by flutter/flutter#130513

Part of flutter/flutter#127030
ditman
ditman approved these changes Jul 27, 2023
View reviewed changes
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.

I still like this after the latest changes. LGTM!

@mdebbar mdebbar added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 7, 2023
@auto-submit auto-submit bot merged commit 55fe41b into flutter:master Aug 7, 2023
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 8, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 8, 2023
@engine-flutter-autoroll
55fe41b [web] New HtmlElementView.fromTagName constructor (flutter/fl…
39f8b9b 
…utter#130513)
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Aug 8, 2023
@engine-flutter-autoroll
Roll Flutter from ad0aa8d to 436df69 (17 revisions) (#4663)
ac41376 
flutter/flutter@ad0aa8d...436df69

2023-08-08 [email protected] Roll Flutter Engine from 9c83d90b01bd to 146c4c9487fc (6 revisions) (flutter/flutter#132112)
2023-08-08 [email protected] Roll Flutter Engine from c27109291e22 to 9c83d90b01bd (5 revisions) (flutter/flutter#132108)
2023-08-08 [email protected] Roll Flutter Engine from be085f6699b6 to c27109291e22 (3 revisions) (flutter/flutter#132086)
2023-08-08 [email protected] Revert "Replace TextField.canRequestFocus with TextField.focusNode.canRequestFocus" (flutter/flutter#132104)
2023-08-08 [email protected] [Impeller] add drawVertices and drawAtlas benchmarks. (flutter/flutter#132080)
2023-08-07 [email protected] Adds more documentations around ignoreSemantics deprecations. (flutter/flutter#131287)
2023-08-07 [email protected] [web] New HtmlElementView.fromTagName constructor (flutter/flutter#130513)
2023-08-07 [email protected] Move mock canvas to flutter_test (flutter/flutter#131631)
2023-08-07 [email protected] Add static_path_tessellation macrobenchmark (flutter/flutter#131837)
2023-08-07 [email protected] [web] Remove usage of `ui.webOnlyInitializePlatform()` (flutter/flutter#131344)
2023-08-07 [email protected] Roll Flutter Engine from 39a575f65d50 to be085f6699b6 (1 revision) (flutter/flutter#132069)
2023-08-07 [email protected] Android context menu theming and visual update (flutter/flutter#131816)
2023-08-07 [email protected] Roll Flutter Engine from 39ce1c097bce to 39a575f65d50 (2 revisions) (flutter/flutter#132064)
2023-08-07 [email protected] CupertinoContextMenu improvement (flutter/flutter#131030)
2023-08-07 [email protected] Roll Flutter Engine from 5b47c0577060 to 39ce1c097bce (3 revisions) (flutter/flutter#132057)
2023-08-07 [email protected] Slider should check `mounted` before start interaction (flutter/flutter#132010)
2023-08-07 [email protected] Roll Packages from ce53da1 to d7ee75a (7 revisions) (flutter/flutter#132058)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
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 Packages: 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
@mdebbar mdebbar deleted the built_in_factory branch August 9, 2023 18:29
gaaclarke pushed a commit to gaaclarke/engine that referenced this pull request Aug 30, 2023
@mdebbar @gaaclarke
[web] Provide convenient default factories for platform views (flutte…
d002850 
…r#43828)

Convenient default factories for creating DOM element from a given tag name.

Required by flutter/flutter#130513

Part of flutter/flutter#127030
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 15, 2023
@engine-flutter-autoroll
55fe41b [web] New HtmlElementView.fromTagName constructor (flutter/fl…
c5b723f 
…utter#130513)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ditman ditman ditman approved these changes

Assignees
No one assigned
Labels
autosubmit Merge PR when tree becomes green via auto submit App framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[web] HtmlElementView improvements
2 participants
@mdebbar @ditman