sfirke commented on code in PR #43940:
URL: https://github.com/apache/superset/pull/43940#discussion_r3969162081
##########
superset-frontend/src/dashboard/components/gridComponents/TabsRenderer/TabsRenderer.test.tsx:
##########
@@ -263,5 +264,117 @@ describe('TabsRenderer', () => {
expect(container).toHaveStyleRule('cursor', 'move', {
target: '.dragdroppable-tab *',
});
+
+ // Release the pointer so the drag does not outlive this test. dnd-kit
+ // keeps swallowing clicks on the shared document for 50ms after a drag
+ // ends, which would eat the tab click of whichever test runs next.
+ fireEvent.pointerUp(document, { button: 0, isPrimary: true, clientX: 50 });
+ await sleep(60);
+ });
+
+ // jsdom's cascade ignores specificity, so assert on the emotion rule rather
+ // than the computed style, which antd's own `position: relative` would win
+ const TAB_BAR = { target: /> ?\.ant-tabs ?> ?\.ant-tabs-nav$/ };
+
+ test('pins the tab bar below the offset supplied by the dashboard', () => {
+ render(
+ <StickyTabsOffsetContext.Provider value={64}>
+ <TabsRenderer {...mockProps} />
+ </StickyTabsOffsetContext.Provider>,
+ );
+ const container = screen.getByTestId('dashboard-component-tabs');
+
+ expect(container).toHaveStyleRule('position', 'sticky', TAB_BAR);
+ expect(container).toHaveStyleRule('top', '64px', TAB_BAR);
+ });
+
+ test('leaves the tab bar in document flow without a dashboard offset', () =>
{
+ render(<TabsRenderer {...mockProps} />);
+ const container = screen.getByTestId('dashboard-component-tabs');
+
+ expect(container).not.toHaveStyleRule('position', 'sticky', TAB_BAR);
+ });
+
+ test('leaves the tab bar in document flow in edit mode', () => {
+ render(
+ <StickyTabsOffsetContext.Provider value={64}>
+ <TabsRenderer {...mockProps} editMode />
+ </StickyTabsOffsetContext.Provider>,
+ );
+ const container = screen.getByTestId('dashboard-component-tabs');
+
+ expect(container).not.toHaveStyleRule('position', 'sticky', TAB_BAR);
+ });
+
+ test('stacks nested tab bars beneath its own tab bar', () => {
Review Comment:
Good catch, this was a genuine coverage gap — fixed in a2cd84e.
The behavior itself was already correct: `measure` is wired as the
`ResizeObserver` callback, so a reflow calls `setTabBarHeight`, which
re-renders and pushes a fresh `childStickyOffset` through the context provider.
But the test never proved it. Our shared jsdom shim
(`spec/helpers/ResizeObserver.ts`) is a no-op whose `observe()` returns `null`,
so `stacks nested tab bars beneath its own tab bar` only ever exercised the
initial synchronous `measure()`.
I added `restacks nested tab bars when its own tab bar reflows`, which
stands in an observer that hands its callback back to the test, asserts the
initial `104`, then grows the measured bar to 80 and fires the callback to
assert the descendant offset follows to `144`. I checked it fails for the right
reason by breaking the callback wiring locally.
One correction to the bot's suggestion for anyone following along: `measure`
ignores the observer entries and re-reads `tabBar.offsetHeight`, so the test
has to move the `offsetHeight` spy and *then* invoke the callback — passing a
new `DOMRect` in wouldn't have asserted anything.
While in there I also guarded the observer construction the way the sticky
header's own effect in `DashboardBuilder` does, so an environment without
`ResizeObserver` keeps the height measured at mount rather than throwing.
---
🤖 _Drafted by Claude Code, co-signed by @sfirke._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
