Skip to content

[beta] CP request for https://github.com/flutter/flutter/pull/167677#168386

Merged
auto-submit[bot] merged 1 commit intoflutter:flutter-3.32-candidate.0from
elliette:beta-cp-167677
May 6, 2025
Merged

[beta] CP request for https://github.com/flutter/flutter/pull/167677#168386
auto-submit[bot] merged 1 commit intoflutter:flutter-3.32-candidate.0from
elliette:beta-cp-167677

Conversation

@elliette
Copy link
Member

@elliette elliette commented May 6, 2025 •
edited
Loading

CP request for #167677 into flutter-3.32-candidate.0

Impacted Users: Some subset of widget inspector users with a specific configuration of package:go_router, see upvotes and comments on #166118.

Impact Description: Depending on how users have configured their routes using package:go_router, enabling / disabling the widget inspector can be destructive to their app's routing state, preventing them from inspecting widgets on secondary screens of their app.

Workaround: No workaround available.

Risk: Low

Test Coverage: Yes, tests were added and this has been manually tested as well

Validation Steps:

  • Run a Flutter app
  • Open the DevTools Inspector for the running app
  • Toggle "Select widget mode"
  • An additional button has been added to the on-device inspector that allows developers to both interact with their app (e.g. navigate to a new page) and select widgets while in Widget Selection mode. See gif below.

new_on_device_inspector

@elliette
[Widget Inspector] Add on-device button for changing default "tap" be…
bd3e9a0 
…havior (flutter#167677)

Fixes flutter#166118

Adds a second button to the on-device inspector that allows a developer
to temporarily disable the default "tap triggers widget selection"
behavior without fully exiting out of widget selection mode. Fixes an
issue where enabling/disabling widget selection is destructive for some
app set-ups, causing the developer's app to lose state.

![new_widget_selection_gif](https://github.com/user-attachments/assets/4f2e96fe-2478-4254-8fe5-4023598f1a5e)

<img width="130" alt="Screenshot 2025-04-23 at 12 22 41 PM"
src="proxy.php?url=https://github.com/user-attachments/assets/e643fd26-6bcb-43a2-a718-191e1831345f"
/>

<img width="113" alt="Screenshot 2025-04-23 at 12 22 48 PM"
src="proxy.php?url=https://github.com/user-attachments/assets/5ac81455-64f0-4f07-9b36-b8fd498a9669"
/>

<img width="105" alt="Screenshot 2025-04-23 at 12 21 46 PM"
src="proxy.php?url=https://github.com/user-attachments/assets/49c67f6f-7d90-4758-83e9-ed8bf5505bae"
/>

<img width="108" alt="Screenshot 2025-04-23 at 12 21 55 PM"
src="proxy.php?url=https://github.com/user-attachments/assets/aeca0178-872d-441e-ae5f-e9b469d83d60"
/>

<img width="399" alt="Screenshot 2025-04-22 at 2 21 46 PM"
src="proxy.php?url=https://github.com/user-attachments/assets/0ad45cee-15cf-45af-9fa0-c0955296aa29"
/>

<img width="134" alt="Screenshot 2025-04-23 at 12 20 19 PM"
src="proxy.php?url=https://github.com/user-attachments/assets/9b34a6c2-5308-465e-b842-0fb180d25865"
/>

<img width="123" alt="Screenshot 2025-04-23 at 12 20 26 PM"
src="proxy.php?url=https://github.com/user-attachments/assets/7c601831-91c8-440e-98ae-070444574ff9"
/>

<img width="106" alt="Screenshot 2025-04-22 at 2 21 38 PM"
src="proxy.php?url=https://github.com/user-attachments/assets/517839d5-25d8-42e7-a9b9-f35a77725afc"
/>

<img width="103" alt="Screenshot 2025-04-22 at 2 21 55 PM"
src="proxy.php?url=https://github.com/user-attachments/assets/390f0b0f-1a9b-4880-b686-fabe102fe7b6"
/>
@flutter-dashboard
Copy link

flutter-dashboard bot commented May 6, 2025

This pull request was opened from and to a release candidate branch. This should only be done as part of the official Flutter release process. If you are attempting to make a regular contribution to the Flutter project, please close this PR and follow the instructions at Tree Hygiene for detailed instructions on contributing to Flutter.

Reviewers: Use caution before merging pull requests to release branches. Ensure the proper procedure has been followed.

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. f: cupertino flutter/packages/flutter/cupertino repository labels May 6, 2025
@elliette elliette marked this pull request as ready for review May 6, 2025 16:53
@reidbaker reidbaker requested review from camsim99 and matanlurey May 6, 2025 17:00
@reidbaker reidbaker added the cp: review Cherry-picks in the review queue label May 6, 2025
@zanderso
Copy link
Member

zanderso commented May 6, 2025

I think this CP should have a reviewer from the framework team given the changes it contains in the framework, and the original PR didn't have a reviewer from that team. cc @Piinks

@matanlurey matanlurey requested review from Piinks and removed request for camsim99 and matanlurey May 6, 2025 17:13
Piinks
Piinks reviewed May 6, 2025
View reviewed changes
packages/flutter/lib/src/cupertino/app.dart
Widget _exitWidgetSelectionButtonBuilder(
BuildContext context, {
required VoidCallback onPressed,
required String semanticLabel,
Copy link
Contributor

@Piinks Piinks May 6, 2025

Choose a reason for hiding this comment

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

FYI for future changes, we're desperately trying to align on semantic*s*Label across the framework. This is private so we can fix it pretty easily with the work we're already doing on this later.

Copy link
Member Author

@elliette elliette May 6, 2025

Choose a reason for hiding this comment

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

Ah good to know, thanks!

Piinks
Piinks reviewed May 6, 2025
View reviewed changes
packages/flutter/lib/src/material/app.dart
import 'scaffold.dart' show ScaffoldMessenger, ScaffoldMessengerState;
import 'scrollbar.dart';
import 'theme.dart';
import 'theme_data.dart';
Copy link
Contributor

@Piinks Piinks May 6, 2025

Choose a reason for hiding this comment

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

theme.dart exports theme_data.dart, I don't think this is necessary.
Nitty, can clean up in a separate change.

Piinks
Piinks reviewed May 6, 2025
View reviewed changes
packages/flutter/lib/src/widgets/widget_inspector.dart
/// Returns [buttonSize] if the variant is [InspectorButtonVariant.iconOnly],
/// otherwise returns [buttonIconSize].
double get iconSizeForVariant =>
variant == InspectorButtonVariant.iconOnly ? buttonSize : buttonIconSize;
Copy link
Contributor

@Piinks Piinks May 6, 2025

Choose a reason for hiding this comment

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

This should be an exhaustive switch statement in case more variants are added in the future.

Piinks
Piinks approved these changes May 6, 2025
View reviewed changes
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 - nits below don't appear necessary for this CP, but can you can follow up with them later? 🙏

packages/flutter/lib/src/material/app.dart
foregroundColor: foreground,
backgroundColor: background,
side:
variant == InspectorButtonVariant.toggle && !toggledOn!
Copy link
Contributor

@Piinks Piinks May 6, 2025

Choose a reason for hiding this comment

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

Same here re: exhaustive switches

@elliette
Copy link
Member Author

elliette commented May 6, 2025

LGTM - nits below don't appear necessary for this CP, but can you can follow up with them later? 🙏

Yes, will open up a PR against main to clean these up. Thank you!

@elliette elliette requested review from camsim99 and matanlurey May 6, 2025 19:23
@matanlurey matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App label May 6, 2025
@auto-submit auto-submit bot merged commit 8545480 into flutter:flutter-3.32-candidate.0 May 6, 2025
143 checks passed
@elliette elliette mentioned this pull request May 7, 2025
Merged
github-merge-queue bot pushed a commit that referenced this pull request May 8, 2025
@elliette
[Widget Inspector] Clean-up changes in PR #167677 (#168488)
83082c1 
Follow-up to #167677

Addresses clean-up comments in cherry-pick:
#168386
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025
@engine-flutter-autoroll
8545480 [beta] CP request for flutter/flutter#167677 (flutter/flutter…
a069245 
…#168386)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025
@engine-flutter-autoroll
8545480 [beta] CP request for flutter/flutter#167677 (flutter/flutter…
c4ab995 
…#168386)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 21, 2025
@engine-flutter-autoroll
8545480 [beta] CP request for flutter/flutter#167677 (flutter/flutter…
938dd39 
…#168386)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@matanlurey matanlurey matanlurey approved these changes

@Piinks Piinks Piinks approved these changes

@camsim99 camsim99 Awaiting requested review from camsim99

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App cp: review Cherry-picks in the review queue f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. 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.

5 participants

@elliette @zanderso @matanlurey @Piinks @reidbaker