fix(sqllab): truncate long tab names in the overflow ("...") dropdown#41585
Conversation
Code Review Agent Run #5db754Actionable Suggestions - 0Additional Suggestions - 1
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
| hideAdd={offline} | ||
| onTabClick={onTabClicked} | ||
| onEdit={handleEdit} | ||
| more={{ overlayClassName: SQLLAB_TAB_OVERFLOW_POPUP_CLASS }} |
There was a problem hiding this comment.
Suggestion: The overflow dropdown class is being passed through more.overlayClassName, but Ant Design Tabs uses popupClassName for the collapsed-tabs menu class. With the current prop shape, the custom class is not applied to the popup, so the global truncation styles never take effect and long tab names can still render blank in the overflow menu. Pass the class through the Tabs popup class prop that the component actually consumes. [api mismatch]
Severity Level: Critical 🚨
- ❌ SQL Lab overflowed tab dropdown still omits long tab titles.
- ⚠️ Users cannot easily switch to hidden long-named tabs.
- ⚠️ SQL Lab tab UX remains broken despite added global styles.Steps of Reproduction ✅
1. Open SQL Lab, which renders `TabbedSqlEditors` via
`src/SqlLab/components/App/index.tsx:6-16` where <TabbedSqlEditors /> is nested inside
`SqlLabStyles` and `AppLayout`.
2. In `TabbedSqlEditors` (`src/SqlLab/components/TabbedSqlEditors/index.tsx:191-208`),
note that each tab item label is a `SqlEditorTabHeader` React node, and the tabs are
rendered through `StyledEditableTabs` (`EditableTabs` wrapper around Ant Design Tabs).
3. The global overflow dropdown truncation styles are defined in
`src/SqlLab/SqlLabGlobalStyles.tsx:4-7,20-43` using the CSS class name
`SQLLAB_TAB_OVERFLOW_POPUP_CLASS = 'sqllab-tab-overflow-popup'`, targeting
`.sqllab-tab-overflow-popup .ant-tabs-dropdown-menu-item` and related selectors.
4. The tabs component attempts to apply that class by passing `more={{ overlayClassName:
SQLLAB_TAB_OVERFLOW_POPUP_CLASS }}` to `StyledEditableTabs`
(`src/SqlLab/components/TabbedSqlEditors/index.tsx:256-33), but `EditableTabs` in
`packages/superset-ui-core/src/components/Tabs/Tabs.tsx:33-47` simply forwards its props
to `AntdTabs` and does not interpret a `more` prop.
5. Ant Design Tabs (as seen from its `AntdTabsProps` usage in `Tabs.tsx:21,25`) exposes
`popupClassName` on the Tabs component, not a `more.overlayClassName` config object;
because `more` is not a valid prop, the overflow dropdown never receives the
`sqllab-tab-overflow-popup` class, the global CSS in `SqlLabGlobalStyles.tsx` does not
apply, and overflowed tabs with very long names continue to render with effectively blank
labels in the "..." dropdown.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.tsx
**Line:** 266:266
**Comment:**
*Api Mismatch: The overflow dropdown class is being passed through `more.overlayClassName`, but Ant Design Tabs uses `popupClassName` for the collapsed-tabs menu class. With the current prop shape, the custom class is not applied to the popup, so the global truncation styles never take effect and long tab names can still render blank in the overflow menu. Pass the class through the Tabs popup class prop that the component actually consumes.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
The flagged issue is correct. The Here is the corrected implementation for onTabClick={onTabClicked}
onEdit={handleEdit}
popupClassName={SQLLAB_TAB_OVERFLOW_POPUP_CLASS}
type={queryEditors?.length === 0 ? 'card' : 'editable-card'}I have checked the PR comments, and there are no other actionable suggestions or pending review threads to address. Would you like me to perform any other checks on this PR? superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.tsx |
| expect(queryByText(extraQueryEditor2.name)).not.toBeInTheDocument(); | ||
| }); | ||
|
|
||
| test('exposes the name on a dedicated node the overflow dropdown can truncate', () => { |
There was a problem hiding this comment.
maybe this can be migrate away from the parent describe block? :D
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Code Review Agent Run #801910Actionable Suggestions - 0Additional Suggestions - 1
Filtered by Review RulesBito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
The SQL Lab tab label is a flex node (icon menu + title + status icon), not plain text. antd's overflow dropdown styles each menu item for a plain-text label (overflow:hidden + text-overflow:ellipsis on the item), so the nested flex defeats the ellipsis and very long tab names render blank, leaving overflowed tabs unreachable from the menu. Scope a style to the overflow dropdown (via the tabs `more` overlayClassName) that caps the menu item width and lets the title truncate inside it. The dropdown is portaled to the body, outside the tabs' emotion scope, so the rule lives in SqlLabGlobalStyles. Claude-Session: https://claude.ai/code/session_015i9w3EMM6EY647fa6SBBSS
Address review: the overflow dropdown class was passed via `more.overlayClassName`; switch to Ant Design Tabs' `popupClassName`, the idiomatic prop for classing the collapsed-tabs popup, so it composes with the internal dropdown classes instead of replacing them. Also flatten SqlEditorTabHeader.test.tsx away from its describe blocks into top-level test() cases (per the test convention), replacing the shared beforeEach with an openTabDropdown() helper.
dcf6df9 to
2cfd3f0
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #41585 +/- ##
==========================================
- Coverage 64.45% 64.44% -0.01%
==========================================
Files 2670 2670
Lines 147335 147336 +1
Branches 33988 33988
==========================================
- Hits 94963 94953 -10
- Misses 50645 50656 +11
Partials 1727 1727
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Bito Automatic Review Skipped – PR Already Merged |
3 participants
SUMMARY
When a SQL Lab tab has a very long name and the tab bar collapses tabs into the overflow ("...") dropdown, the dropdown entries render blank, so overflowed tabs become unreachable.
The tab
labelis a flex node (SqlEditorTabHeader: icon menu + title + status icon), not plain text. antd styles each overflow menu item for a plain-text label (overflow:hidden+text-overflow:ellipsison the item,flex:1on its> span). The nested flexTabTitleWrapperdefeats that ellipsis and, with no width bound, an extremely long name overflows the item and gets clipped out of view.Fix: scope a style to the overflow dropdown (via the tabs
moreoverlayClassName) that caps the menu-item width and lets the title truncate inside it. The dropdown is portaled tobody, outside the tabs' emotion scope, so the rule lives inSqlLabGlobalStyles. The tab bar itself is untouched.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before: overflow dropdown rows for long-named tabs show empty space (no name).
After: each row shows the name truncated with an ellipsis, e.g.
AAAAAAAA…, with the menu icon and status dot still visible.TESTING INSTRUCTIONS
Unit:
npm run test -- src/SqlLab/components/SqlEditorTabHeader/SqlEditorTabHeader.test.tsxADDITIONAL INFORMATION
https://claude.ai/code/session_015i9w3EMM6EY647fa6SBBSS