Repository navigation
Add a platform override dropdown to the Inspector toolbar - #10061
Open
Aadeesh-01 wants to merge 2 commits into
Open
Aadeesh-01 wants to merge 2 commits into
Aadeesh-01 wants to merge 2 commits into
Conversation
Aadeesh-01
requested review from
a team,
kenzieschmoll and
srawlins
as code owners
October 31, 2026 17:57
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a new PlatformOverrideDropdown widget to the DevTools Inspector toolbar, allowing users to override the target platform of a connected Flutter app. It also adds 'windows' to the supported platforms and updates the relevant tests, release notes, and changelogs. The feedback suggests aligning the implementation with the DevTools Style Guide by using MultiValueListenableBuilder instead of nested ValueListenableBuilders, and replacing unawaited with safeUnawaited for fire-and-forget futures.
Comment on lines
+1238
to
+1265
| return ValueListenableBuilder<bool>( | ||
| valueListenable: extensionManager.hasServiceExtension(extensionName), | ||
| builder: (context, available, _) { | ||
| if (!available) return const SizedBox.shrink(); | ||
| return ValueListenableBuilder<ServiceExtensionState>( | ||
| valueListenable: extensionManager.getServiceExtensionState( | ||
| extensionName, | ||
| ), | ||
| builder: (context, state, _) { | ||
| // The value is null until DevTools has read it from the app. | ||
| final current = state.value as String?; | ||
| return AnalyticsDropDownButton<String>( | ||
| key: dropdownKey, | ||
| gaScreen: gac.inspector, | ||
| gaDropDownId: gac.togglePlatform, | ||
| message: togglePlatformMode.tooltip, | ||
| isDense: true, | ||
| value: current, | ||
| items: [ | ||
| for (final platform in _platformsToShow(current)) | ||
| _buildMenuItem(platform), | ||
| ], | ||
| onChanged: _onPlatformChanged, | ||
| ); | ||
| }, | ||
| ); | ||
| }, | ||
| ); |
Contributor
There was a problem hiding this comment.
[CONCERN] Use the MultiValueListenableBuilder helper instead of nesting ValueListenableBuilders, as recommended by the DevTools Style Guide.
return MultiValueListenableBuilder(
listenables: [
extensionManager.hasServiceExtension(extensionName),
extensionManager.getServiceExtensionState(extensionName),
],
builder: (context, values, _) {
final available = values[0] as bool;
if (!available) return const SizedBox.shrink();
final state = values[1] as ServiceExtensionState;
// The value is null until DevTools has read it from the app.
final current = state.value as String?;
return AnalyticsDropDownButton<String>(
key: dropdownKey,
gaScreen: gac.inspector,
gaDropDownId: gac.togglePlatform,
message: togglePlatformMode.tooltip,
isDense: true,
value: current,
items: [
for (final platform in _platformsToShow(current))
_buildMenuItem(platform),
],
onChanged: _onPlatformChanged,
);
},
);References
- Prefer MultiValueListenableBuilder instead of nested ValueListenableBuilders. (link)
Comment on lines
+1316
to
+1323
| unawaited( | ||
| serviceConnection.serviceManager.serviceExtensionManager | ||
| .setServiceExtensionState( | ||
| togglePlatformMode.extension, | ||
| enabled: true, | ||
| value: platform, | ||
| ), | ||
| ); |
Contributor
There was a problem hiding this comment.
[CONCERN] Use safeUnawaited instead of unawaited for fire-and-forget futures, as recommended by the DevTools Style Guide.
safeUnawaited(
serviceConnection.serviceManager.serviceExtensionManager
.setServiceExtensionState(
togglePlatformMode.extension,
enabled: true,
value: platform,
),
);References
- Prefer safeUnawaited(...) for fire-and-forget futures. (link)
This branch has not been deployed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #2780
Adds a dropdown to the Flutter Inspector toolbar that overrides the target platform of the connected app, using the existing
ext.flutter.platformOverrideservice extension (togglePlatformMode). This brings back the platform switch that the IntelliJ Inspector had.Changes
PlatformOverrideDropdowninservice_extension_widgets.dart, built onAnalyticsDropDownButton(dense, with an icon and name for each platform).getServiceExtensionStateand sets it withsetServiceExtensionState, so it stays in sync when the app changes the platform itself.hasServiceExtensionis false, for example in profile or release mode.TogglePlatformSelectordid.DropdownButtondoes not throw.InspectorServiceExtensionButtonGroup, before the overlay buttons.windowstotogglePlatformModevalues indevtools_app_shared, with a CHANGELOG entry. Without it, after switching a Windows app to another platform there was no way to switch back.iOS,Android,macOS, … because the icon and tooltip already give context. This also fixesMacOStomacOS.TODO(#2780)andignore: unused-codecomments ontogglePlatformMode.Screenshots
Questions for reviewers
ServiceExtensionManagertreats multi-value extensions as enabled, so a "Default" entry would not stay selected. Picking your own platform works as a reset. I'm happy to add a default option if you have an approach you prefer.ServiceExtensionSelector/TogglePlatformSelectorblock inservice_extension_widgets.dart. This PR is the Flutter version that its TODO asks for, so I can delete it here if you'd like.Tests
test/service/service_extension_widgets_test.dart: switching the platform calls the extension with the new value; Windows is listed and Fuchsia is not; an unknown platform reported by the app is listed; the dropdown is hidden when the extension is unavailable.test/screens/inspector/inspector_screen_test.dart: the dropdown appears in the Inspector toolbar.Pre-launch Checklist
General checklist
///).Issues checklist
contributions-welcomeorgood-first-issuelabel.contributions-welcomeorgood-first-issuelabel. I understand this means my PR might take longer to be reviewed.Tests checklist
AI-tooling checklist
Feature-change checklist
release-notes-not-requiredlabel or left a comment requesting the label be added.packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md.If you need help, consider asking for help on Discord.
🤖 Generated with Claude Code