This repository was archived by the owner on Feb 25, 2025. It is now read-only.
[web] Provide convenient default factories for platform views #43828
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ditman
reviewed
Jul 21, 2023
Member
ditman
left a comment
ditman
left a comment
There was a problem hiding this comment.
This looks great! I think that if we exposed the constants of the default factory names from package:ui_web, the implementation would be slightly more robust (one less thing to have to worry about I guess?)
|
|
||
| typedef _DefaultFactoryParams = Map<Object?, Object?>; | ||
|
|
||
| extension on _DefaultFactoryParams { |
Member
There was a problem hiding this comment.
What a wacky syntax we can get with extensions, I love it (in small doses) xD
Contributor
Author
There was a problem hiding this comment.
I actually removed this because I was able to do the same thing using the existing readX extensions:
engine/lib/web_ui/lib/src/engine/util.dart
Line 560 in e590b24
Contributor
Author
|
@ditman this is ready for a re-review! |
ditman
approved these changes
Jul 25, 2023
Member
ditman
left a comment
ditman
left a comment
There was a problem hiding this comment.
Small comment about encapsulating the currently global consts inside the class that "owns" them, as static finals, if possible!
mdebbar
added
the
autosubmit
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jul 26, 2023
auto-submit bot
pushed a commit
to flutter/flutter
that referenced
this pull request
Jul 26, 2023
…131339) flutter/engine@ba83c14...faf1121 2023-07-26 [email protected] Roll Skia from 5ace549dfae6 to ca48e45a0262 (4 revisions) (flutter/engine#44038) 2023-07-26 [email protected] [web] Provide convenient default factories for platform views (flutter/engine#43828) 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
ditman
reviewed
Jul 27, 2023
| /// | ||
| /// There's no need to register this view type with [PlatformViewRegistry] | ||
| /// because it is registered by default. | ||
| static const String defaultVisibleViewType = '_default_document_create_element_visible'; |
LouiseHsu
pushed a commit
to LouiseHsu/flutter
that referenced
this pull request
Jul 31, 2023
…lutter#131339) flutter/engine@ba83c14...faf1121 2023-07-26 [email protected] Roll Skia from 5ace549dfae6 to ca48e45a0262 (4 revisions) (flutter/engine#44038) 2023-07-26 [email protected] [web] Provide convenient default factories for platform views (flutter/engine#43828) 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
vashworth
pushed a commit
to vashworth/flutter
that referenced
this pull request
Aug 2, 2023
…lutter#131339) flutter/engine@ba83c14...faf1121 2023-07-26 [email protected] Roll Skia from 5ace549dfae6 to ca48e45a0262 (4 revisions) (flutter/engine#44038) 2023-07-26 [email protected] [web] Provide convenient default factories for platform views (flutter/engine#43828) 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
Aug 7, 2023
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
gaaclarke
pushed a commit
to gaaclarke/engine
that referenced
this pull request
Aug 30, 2023
…r#43828) Convenient default factories for creating DOM element from a given tag name. Required by flutter/flutter#130513 Part of flutter/flutter#127030
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Convenient default factories for creating DOM element from a given tag name.
Required by flutter/flutter#130513
Part of flutter/flutter#127030