-
Notifications
You must be signed in to change notification settings - Fork 345
Allow switching between legacy and new Inspector #8342
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
Conversation
DBC: if we want to increase our chances of users using the V2 inspector to gather early feedback, wdyt about pulling the setting out into the top level row of actions as a Switch "Try the new Inspector!"? |
packages/devtools_app/lib/src/screens/inspector/inspector_tree_controller.dart
Outdated
Show resolved
Hide resolved
packages/devtools_app/lib/src/screens/inspector_shared/inspector_screen.dart
Outdated
Show resolved
Hide resolved
packages/devtools_app/lib/src/screens/inspector_shared/inspector_screen.dart
Outdated
Show resolved
Hide resolved
packages/devtools_app/lib/src/screens/inspector_shared/inspector_screen_controller.dart
Outdated
Show resolved
Hide resolved
packages/devtools_app/lib/src/screens/inspector_shared/inspector_screen_controller.dart
Outdated
Show resolved
Hide resolved
packages/devtools_app/lib/src/screens/inspector_shared/inspector_settings_dialog.dart
Show resolved
Hide resolved
packages/devtools_app/lib/src/shared/preferences/_inspector_preferences.dart
Outdated
Show resolved
Hide resolved
packages/devtools_app/lib/src/shared/preferences/_inspector_preferences.dart
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
super-nit: can we take a screenshot without the debug label in the upper left? most users will experience this in the embedded view anyway so perhaps add the embedMode=single
query param and then take a screenshot?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! Will update the screenshots in the release notes as a last step once everything else in the PR looks good
if (controller != null) ...[ | ||
const SizedBox(width: defaultSpacing), | ||
ShowImplementationWidgetsButton(controller: controller!), | ||
], | ||
const Spacer(), | ||
// TODO(https://github.com/flutter/devtools/issues/7860): Clean-up after | ||
// Inspector V2 has been released. | ||
if (FeatureFlags.inspectorV2) ...[ | ||
const SizedBox(width: defaultSpacing), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could this all be in the same spread?
if (controller != null && FeatureFlags.inspectorV2) ...[
const SizedBox(width: defaultSpacing),
ShowImplementationWidgetsButton(controller: controller!),
const Spacer(),
const SizedBox(width: defaultSpacing),
SwitchSetting(...),
] else
const Spacer(),
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately it can't! The existence of controller
means we are on new Inspector, but even for the legacy Inspector we need to show the SwitchSetting
(so that users can enable the new Inspector)
Fixes #8269
Allow users to switch between the legacy inspector and the new inspector.
Previously the feature flag
inspectorV2
determined whether to show the legacyInspectorScreen
or the newInspectorScreen
.Now, the same
InspectorScreen
(inspector_shared/inspector_screen.dart
) is always shown, and the user preferenceenableInspectorV2
determines whether it builds the legacyInspectorScreenBody
or the newInspectorScreenBody
.Note: The
InspectorSettingsDialog
which had two implementations in the legacyinspector/
directory and the newinspector_v2/
directory now have been combined and also live ininspector_shared/
.