feat(sqllab): improve table metadata UI #32051
Conversation
dosubot
bot
added
change:frontend
github-actions
bot
added
packages
and removed
sqllab
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
| Category | Issue | Fix Detected |
|---|---|---|
 |
Incorrect South Pane Tab Activation ▹ view | |
|
Incomplete table object in removal function ▹ view | ✅ |
 |
Unclear CSS Selector Exclusion ▹ view | |
|
Breaking Change: Columns Property Now Required ▹ view |
Need a new review? Comment
/korbit-reviewon this PR and I'll review your latest changes.Korbit Guide: Usage and Customization
Interacting with Korbit
- You can manually ask Korbit to review your PR using the
/korbit-reviewcommand in a comment at the root of your PR.- You can ask Korbit to generate a new PR description using the
/korbit-generate-pr-descriptioncommand in any comment on your PR.- Too many Korbit comments? I can resolve all my comment threads if you use the
/korbit-resolvecommand in any comment on your PR.- Chat with Korbit on issues we post by tagging @korbit-ai in your reply.
- Help train Korbit to improve your reviews by giving a 👍 or 👎 on the comments Korbit posts.
Customizing Korbit
- Check out our docs on how you can make Korbit work best for you and your team.
- Customize Korbit for your organization through the Korbit Console.
Feedback and Support
| columns: { | ||
| name: string; | ||
| type: string; | ||
| longType: string; | ||
| }[]; |
There was a problem hiding this comment.
Breaking Change: Columns Property Now Required 
Tell me more
What is the issue?
The columns property has been changed from optional (columns?) to required (columns), which could cause runtime errors for existing data that might not have the columns property defined.
Why this matters
Existing table schemas in the system that don't have the columns property will now fail type checking and could cause the application to crash when attempting to access this required property.
Suggested change ∙ Feature Preview
Keep the columns property optional to maintain backward compatibility:
columns?: {
name: string;
type: string;
longType: string;
}[];
💬 Chat with Korbit by mentioning @korbit-ai.
| const removeTable = useCallback( | ||
| (tableId, action) => { | ||
| if (action === 'remove') { | ||
| const table = { | ||
| id: tableId, | ||
| initialized: true, | ||
| queryEditorId, | ||
| }; | ||
| dispatch(removeTables([table])); | ||
| } | ||
| }, | ||
| [dispatch, queryEditorId], | ||
| ); |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| @@ -47,7 +47,7 @@ const SqlLabStyles = styled.div` | |||
| left: 0; | |||
| padding: 0 ${theme.gridUnit * 2}px; | |||
| pre { | |||
| pre:not(.code) { | |||
There was a problem hiding this comment.
Unclear CSS Selector Exclusion 
Tell me more
What is the issue?
The CSS selector .code is used in a negation without context or explanation for why certain pre elements should be excluded.
Why this matters
Without understanding which pre elements have the .code class and why they need different styling, future maintainers might accidentally break the styling when modifying the codebase.
Suggested change ∙ Feature Preview
Add a comment explaining which pre elements have the .code class and why they need different styling:
/* Exclude pre elements with .code class which are used for syntax highlighting */
pre:not(.code) {💬 Chat with Korbit by mentioning @korbit-ai.
| } | ||
| // for new table, associate Id of query for data preview | ||
| at.dataPreviewQueryId = null; | ||
| let newState = addToArr(state, 'tables', at, Boolean(action.prepend)); | ||
| newState.activeSouthPaneTab = at.id; |
There was a problem hiding this comment.
Incorrect South Pane Tab Activation
Tell me more
What is the issue?
The MERGE_TABLE action unconditionally sets activeSouthPaneTab for new tables, even when the table is not expanded.
Why this matters
This could cause unexpected tab switching in the south pane when tables are added but not meant to be immediately viewed, disrupting user workflow.
Suggested change ∙ Feature Preview
Only set the active tab when the table is expanded:
let newState = addToArr(state, 'tables', at, Boolean(action.prepend));
if (at.expanded) {
newState.activeSouthPaneTab = at.id;
}💬 Chat with Korbit by mentioning @korbit-ai.
|
/testenv up |
SUMMARY
In the case of the treeview for #31591, I discovered that if the number of tables is enormous, it causes performance issues. Therefore, this commit extracted only the modified table preview UI portion by replacing it to output the metadata of the initially selected table. (preview will be the subset of nested tabs under table preview tab)
In this change, the preview query is no longer executed automatically. Instead, it is set to execute the preview when the tab is clicked for verification. Additionally, it has been expanded to allow additional table-related information tabs through extensions, beyond just table columns and index information.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before:
After:
TESTING INSTRUCTIONS
Go to SQL Lab and select table schema options
ADDITIONAL INFORMATION