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
mdebbar wants to merge 3 commits intoflutter:mainfrom
mdebbar:singleton_window_multi_view
Closed

[web] Rename EngineFlutterWindow => EngineSingletonFlutterWindow#45981
mdebbar wants to merge 3 commits intoflutter:mainfrom
mdebbar:singleton_window_multi_view

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
@mdebbar mdebbar mentioned this pull request Sep 18, 2023
Merged
ditman
ditman approved these changes Sep 19, 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.

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.

Reviewers

1 more reviewer

@ditman ditman ditman approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

platform-web Code specifically for the web engine

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mdebbar @ditman