-
Notifications
You must be signed in to change notification settings - Fork 17
feat: keep plan representation tab selection between query executions #2625
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
|
change title according to commitLint: https://github.com/conventional-changelog/commitlint/#what-is-commitlint |
Co-authored-by: adameat <[email protected]>
Updated commit message to follow conventional commit format as required by commitlint. The latest commit (e66e8c5) demonstrates the proper format: |
|
update PR title according to commitLint |
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.
Pull Request Overview
This PR implements tab persistence for the query result viewer, preserving user tab selections (JSON, AST, Explain, etc.) between query executions. Previously, the selected tab would reset to the default after each query run, forcing users to repeatedly select their preferred tab.
Key changes:
- Added Redux state management for tab persistence with separate preferences for execute vs explain queries
- Modified QueryResultViewer to use Redux state instead of local React state
- Added comprehensive test coverage for the new functionality
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/store/reducers/query/types.ts | Added selectedResultTab field to QueryState for storing tab preferences |
| src/store/reducers/query/query.ts | Added setResultTab action and selectResultTab selector for Redux state management |
| src/store/reducers/query/test/tabPersistence.test.tsx | Integration tests for tab persistence functionality |
| src/store/reducers/query/test/resultTab.test.ts | Unit tests for the setResultTab reducer |
| src/containers/Tenant/Query/QueryResult/QueryResultViewer.tsx | Modified component to use Redux state for tab selection with validation and fallback logic |
| @@ -1,4 +1,5 @@ | |||
| import React from 'react'; | |||
| // Query result viewer with tab persistence functionality | |||
Copilot
AI
Jul 28, 2025
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.
[nitpick] This single-line comment doesn't add meaningful documentation value. Consider either removing it or expanding it into a proper JSDoc comment that explains the component's purpose, props, and key functionality.
| // Query result viewer with tab persistence functionality | |
| /** | |
| * QueryResultViewer is a React component that displays the results of a query execution. | |
| * It supports multiple result views (e.g., result sets, schema, JSON, etc.) with tab persistence functionality. | |
| * | |
| * Props: | |
| * - `result` (QueryResult): The query result data to display. | |
| * - `resultType` (QueryAction, optional): The type of query result (default: 'execute'). | |
| * - `isResultsCollapsed` (boolean, optional): Whether the results section is collapsed. | |
| * - `theme` (string, optional): The theme to apply to the component. | |
| * - `tenantName` (string): The name of the tenant associated with the query. | |
| * - `queryText` (string, optional): The text of the executed query. | |
| * - `tableSettings` (Partial<Settings>, optional): Settings for the result table display. | |
| * - `onCollapseResults` (VoidFunction): Callback to collapse the results section. | |
| * - `onExpandResults` (VoidFunction): Callback to expand the results section. | |
| */ |
| const {error, isLoading, data = {}} = result; | ||
| const {preparedPlan, simplifiedPlan, stats, resultSets, ast} = data; | ||
|
|
||
| React.useEffect(() => { |
Copilot
AI
Jul 28, 2025
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.
[nitpick] This useEffect only runs cleanup logic and doesn't depend on any values, but it's placed in the middle of the component logic. Consider moving it closer to other useEffect hooks or to the end of the component for better code organization.
|
@copilot remove |
Co-authored-by: artemmufazalov <[email protected]>
Head branch was pushed to by a user without write access
Removed the duplicate test file |
When users run explain queries and select a tab different from the default "Computation Graph" (such as JSON, AST, or Explain), their selection was not preserved after modifying the query and re-running the explain. This forced users to repeatedly select their preferred tab after each query execution, creating a poor user experience.
Root Cause
The
QueryResultViewercomponent was managing the selected tab (activeSection) as local React state usinguseState. This local state was reset every time the component re-rendered with new query results, causing the user's tab selection to be lost.Solution
Implemented Redux state management to persist the selected tab preference between query executions:
QueryStatewithselectedResultTabfield to track separate tab preferences for 'execute' and 'explain' query typessetResultTabaction to update tab selections andselectResultTabselector to retrieve saved preferencesQueryResultViewerto use Redux state instead of local state, with fallback logic to original defaults when no preference is savedKey Features
Testing
Users can now select their preferred tab for explain queries and it will be remembered across query executions, providing a much smoother development experience.
Fixes #2405.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.
CI Results
Test Status:⚠️ FLAKY
📊 Full Report
Test Changes Summary ⏭️2
⏭️ Skipped Tests (2)
Bundle Size: ✅
Current: 85.29 MB | Main: 85.29 MB
Diff: +1.61 KB (0.00%)
✅ Bundle size unchanged.
ℹ️ CI Information