[Inspector V2] Enable Inspector V2 by default #8650
Conversation
| valueListenable: preferences.inspector.inspectorV2Enabled, | ||
| builder: (context, v2Enabled, _) { | ||
| valueListenable: preferences.inspector.legacyInspectorEnabled, | ||
| builder: (context, legacyInspectorEnabled, _) { |
There was a problem hiding this comment.
if legacyInspectorEnabled is not used, we should use an underscore instead _
| // TODO(https://github.com/flutter/devtools/issues/1423): Default to true | ||
| // after verifying auto-refreshes are performant. | ||
| final _autoRefreshEnabled = ValueNotifier<bool>(true); |
There was a problem hiding this comment.
do we still need this todo? It looks like we are already defaulting to true
There was a problem hiding this comment.
Ah forgot to delete that TODO when I switched auto-refresh on. I've removed it.
| Future<void> _initInspectorV2Enabled() async { | ||
| await _updateInspectorV2Enabled(); | ||
| Future<void> _initLegacyInspectorEnabled() async { | ||
| await _updateLegacyInspectorEnabled(); |
There was a problem hiding this comment.
should remove the old 'inspector.inspectorV2Enabled' key as part of initializing the legacy enabled key?
There was a problem hiding this comment.
I've opened up a bug for that: #8667
We need to add functionality to remove storage keys, which I would like to do in a separate PR. For now, this means that inspector.inspectorV2Enabled will be stored in the browser storage, but we aren't reading it so it shouldn't have any impact.
This PR:
Note: Making this a banner and not a notification because I found a bug with our notification service: #8651