Preserve selected feature tab in Performance page exports#9861
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request adds support for exporting and restoring the selected feature tab index when saving and loading offline performance data. The review feedback highlights a potential logical bug where restoring the tab index without validation could lead to it being overwritten or causing an out-of-bounds crash in the UI. Additionally, it is recommended to add a test case to verify the serialization and deserialization of non-zero tab indices.
The Performance page tracked the selected feature tab via `selectedFeatureTabIndex`, but it was never persisted to offline exports, so loading exported data always reset to the first tab. Mirror the Memory page's existing behavior: serialize the selected tab index into `OfflinePerformanceData` and restore it when loading offline data so the user lands on the tab they exported from. Closes flutter#5150
e35bf0a to
ece7b80
Compare
Address review feedback on the offline tab restoration: - The restored selectedFeatureTabIndex was immediately reset to 0 by the default-feature activation in TabbedPerformanceView. Use the restored index (clamped to the available tabs) when activating the default feature instead of hardcoding 0. - The number of visible tabs can be smaller when loading offline data than when it was exported, so an out-of-range index could be passed to AnalyticsTabbedView. Clamp the index passed as initialSelectedIndex. - Add a round-trip test for a non-zero selectedTab.
Closes #5150
Description
The Performance page tracks the selected feature tab via
PerformanceController.selectedFeatureTabIndex, but that index was never written to offline exports. As a result, loading exported performance data always reset the view to the first tab instead of the tab that was active when the data was exported.This mirrors the behavior the Memory page already implements (
OfflineMemoryData.selectedTab):OfflinePerformanceDatanow has aselectedTabfield that is serialized intoJson/fromJson(defaults to0for backward compatibility with older export files that don't contain the key).prepareOfflineScreenDatawrites the currentselectedFeatureTabIndexinto the export._loadOfflineDatarestoresselectedFeatureTabIndexfrom the loaded data.Tests
performance_model_test.dartto assertselectedTabis created, parsed, and serialized.samplePerformanceDatafixture so the existingtoJsonround-trip test stays in sync.I don't have a Flutter/Dart toolchain set up locally, so I couldn't run
dart analyze/ the test suite myself — relying on CI to validate.