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

[web] Rename EngineFlutterWindow => EngineSingletonFlutterWindow #45981

Closed
wants to merge 3 commits into from

Conversation

mdebbar
Copy link
Contributor

@mdebbar mdebbar commented Sep 18, 2023

Make it clear that this class is a singleton.

Part of flutter/flutter#134443

@mdebbar mdebbar requested a review from ditman September 18, 2023 15:01
@github-actions github-actions bot added the platform-web Code specifically for the web engine label Sep 18, 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.

If you want to land this rename, land it, but I think its utility is going to be limited (or short lived).

Take into account that ui.SingletonFlutterWindow is now deprecated:

  • engine/lib/ui/window.dart

    Lines 397 to 402 in 10c4803

    @Deprecated(
    'Use FlutterView or PlatformDispatcher instead. '
    'Deprecated to prepare for the upcoming multi-window support. '
    'This feature was deprecated after v3.7.0-32.0.pre.'
    )
    class SingletonFlutterWindow extends FlutterView {

Maybe a better approach here is to make class EngineFlutterView extends ui.FlutterView? (that's probably a much bigger change, because we're going to be missing a lot of getters!)

@mdebbar
Copy link
Contributor Author

mdebbar commented Sep 20, 2023

@ditman you are right, this class is going away.

What prompted me to do this rename was your comment. I felt maybe it was not clear that this is a singleton so I wanted to make it clear.

@mdebbar mdebbar closed this Sep 20, 2023
@ditman
Copy link
Member

ditman commented Sep 20, 2023

Ahhh, I got you! I think the current name is "correct", except it should eventually extends ui.FlutterView + our web interface rather than the ui.SingletonFlutterWindow, apologies for miscomunicating this (x_x)

@mdebbar mdebbar deleted the singleton_window_multi_view branch December 11, 2023 21:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
platform-web Code specifically for the web engine
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants