Skip to content

[ Widget Preview ] Improve widget inspector support for widget previews #168013

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.

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

Merged
merged 3 commits into from
May 2, 2025

Conversation

bkonyi
Copy link
Contributor

@bkonyi bkonyi commented Apr 29, 2025

In order to prevent the widget inspector from being able to select and inspect elements of the widget previewer's scaffolding, this commit consists of two notable changes:

  1. A debugWillManuallyInjectWidgetInspector property has been added to WidgetsBinding. Setting this property to true before running the application will prevent WidgetsApp from injecting a WidgetInspector instance into the widget tree, even if the widget inspector is enabled. This means that the widget inspector will not be able to select and highlight widgets by default, requiring WidgetInspector to be manually wrapped around widget trees that should be inspectable.

  2. The widget_preview_scaffold template has been updated to set debugWillManuallyInjectWidgetInspector to true by default, and to wrap individual previews provided by the developer with an instance of WidgetInspector to restrict widget inspection to the contents of the preview.

This change also includes a minor bug fix for situations where WidgetInspector is inserted into an unconstrained context. Previously, the WidgetInspector's _InspectorOverlay would attempt to take up as much space as possible, causing an overflow. To fix this, the _InspectorOverlay is wrapped with Positioned.fill(...) to force it to take on the same size as its parent Stack.

Work towards #166423

Demo:

Screen.Recording.2025-04-29.at.3.50.56.PM.mov

In order to prevent the widget inspector from being able to select and
inspect elements of the widget previewer's scaffolding, this commit
consists of two notable changes:

1) A `debugWillManuallyInjectWidgetInspector` property has been added to
   `WidgetsBinding`. Setting this property to true before running the
   application will prevent `WidgetsApp` from injecting a
   `WidgetInspector` instance into the widget tree, even if the widget
   inspector is enabled. This means that the widget inspector will not
   be able to select and highlight widgets by default, requiring
   `WidgetInspector` to be manually wrapped around widget trees that
   should be inspectable.

2) The widget_preview_scaffold template has been updated to set
   `debugWillManuallyInjectWidgetInspector` to true by default, and to
   wrap individual previews provided by the developer with an instance
   of `WidgetInspector` to restrict widget inspection to the contents of
   the preview.

This change also includes a minor bug fix for situations where
`WidgetInspector` is inserted into an unconstrained context. Previously,
the `WidgetInspector`'s `_InspectorOverlay` would attempt to take up as
much space as possible, causing an overflow. To fix this, the
`_InspectorOverlay` is wrapped with `Positioned.fill(...)` to force it
to take on the same size as its parent `Stack`.

Work towards #166423
@github-actions github-actions bot added tool Affects the "flutter" command-line tool. See also t: labels. framework flutter/packages/flutter repository. See also f: labels. labels Apr 29, 2025
@bkonyi bkonyi requested review from jyameo and Piinks April 29, 2025 20:06
@bkonyi bkonyi marked this pull request as ready for review April 29, 2025 20:06
@bkonyi
Copy link
Contributor Author

bkonyi commented Apr 29, 2025

FYI @Piinks, there's a minor change to the framework needed to support this behavior.

@bkonyi
Copy link
Contributor Author

bkonyi commented Apr 30, 2025

FYI @elliette

Copy link
Contributor

@jyameo jyameo left a comment

Choose a reason for hiding this comment

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

lgtm!

// Verify that the widget inspector is disabled and there's no instances of WidgetInspector
// in the tree.
final Finder widgetInspectorFinder = find.byType(WidgetInspector);
expect(binding.debugShowWidgetInspectorOverride, false);
Copy link
Member

Choose a reason for hiding this comment

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

Why can't we use the existing debugShowWidgetInspectorOverride for widget previews instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We do, but we also need to make sure that debugShowWidgetInspectorOverride == true doesn't result in an instance of WidgetInspector being inserted by WidgetsApp.

This test is simply testing that debugWillManuallyInjectWidgetInspector is able to disable the creation of WidgetInspector by WidgetsApp.

Copy link
Member

Choose a reason for hiding this comment

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

I'm trying to understand if we can set debugShowWidgetInspectorOverride to false to prevent the widget inspector from being injected into the WidgetsApp (how is the behavior of debugShowWidgetInspectorOverride different than debugWillManuallyInjectWidgetInspector)?

Copy link
Contributor Author

@bkonyi bkonyi May 2, 2025

Choose a reason for hiding this comment

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

We can, but the problem is we also want to allow for the manually created WidgetInspector widgets to be removed from the widget tree when widget inspection isn't enabled. That's done by listening to the value of debugShowWidgetInspectorOverride, which DevTools updates through a service extension to enable / disable the inspector. debugExcludeRootWidgetInspector is used to differentiate between the root widget inspector simply being disabled due to widget inspection not being currently enabled vs us never wanting the root widget inspector to be inserted into the widget tree.

Copy link
Contributor

@Piinks Piinks left a comment

Choose a reason for hiding this comment

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

LGTM with the suggested name change!

@bkonyi bkonyi added the autosubmit Merge PR when tree becomes green via auto submit App label May 2, 2025
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 2, 2025
Copy link
Contributor

auto-submit bot commented May 2, 2025

autosubmit label was removed for flutter/flutter/168013, because - The status or check suite Mac tool_integration_tests_1_5 has failed. Please fix the issues identified (or deflake) before re-applying this label.

@bkonyi bkonyi added the autosubmit Merge PR when tree becomes green via auto submit App label May 2, 2025
@auto-submit auto-submit bot added this pull request to the merge queue May 2, 2025
Merged via the queue into master with commit e3db1bb May 2, 2025
151 checks passed
@auto-submit auto-submit bot deleted the widget_inspector_support_previews branch May 2, 2025 19:58
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 2, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 3, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 3, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 4, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 5, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 5, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 5, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 5, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 5, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 6, 2025
mboetger pushed a commit to mboetger/flutter that referenced this pull request May 6, 2025
…ws (flutter#168013)

In order to prevent the widget inspector from being able to select and
inspect elements of the widget previewer's scaffolding, this commit
consists of two notable changes:

1) A `debugWillManuallyInjectWidgetInspector` property has been added to
`WidgetsBinding`. Setting this property to true before running the
application will prevent `WidgetsApp` from injecting a `WidgetInspector`
instance into the widget tree, even if the widget inspector is enabled.
This means that the widget inspector will not be able to select and
highlight widgets by default, requiring `WidgetInspector` to be manually
wrapped around widget trees that should be inspectable.

2) The widget_preview_scaffold template has been updated to set
`debugWillManuallyInjectWidgetInspector` to true by default, and to wrap
individual previews provided by the developer with an instance of
`WidgetInspector` to restrict widget inspection to the contents of the
preview.

This change also includes a minor bug fix for situations where
`WidgetInspector` is inserted into an unconstrained context. Previously,
the `WidgetInspector`'s `_InspectorOverlay` would attempt to take up as
much space as possible, causing an overflow. To fix this, the
`_InspectorOverlay` is wrapped with `Positioned.fill(...)` to force it
to take on the same size as its parent `Stack`.

Work towards flutter#166423

**Demo:**


https://github.com/user-attachments/assets/6d9d384c-5470-4828-983d-a6d9051a2282
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 7, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 7, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 7, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 7, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 7, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 7, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 7, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
framework flutter/packages/flutter repository. See also f: labels. tool Affects the "flutter" command-line tool. See also t: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants