EnxDev commented on code in PR #44345:
URL: https://github.com/apache/superset/pull/44345#discussion_r4080941286
##########
superset-frontend/src/dashboard/components/gridComponents/Tabs/Tabs.test.tsx:
##########
@@ -265,6 +265,144 @@ test('Switching tabs', async () => {
expect(props.onChangeTab).toHaveBeenCalled();
});
+test.each([false, true])(
+ 'A childless TABS component does not register an active tab (editMode=%s)',
+ editMode => {
+ // Regression guard for the permalink failure caused by empty tab
containers:
Review Comment:
Trimmed. One line left here naming the `undefined` -> `null` coercion, since
that is the part not already in the test name; the comment blocks on the other
three new tests are gone.
##########
docs/docs/using-superset/creating-your-first-dashboard.mdx:
##########
@@ -433,6 +433,11 @@ The dropdown menu is briefly hidden while the screenshot
or PDF is being capture
These menu items respect your permissions: the dashboard export menu only
appears if you can download, and the image/PDF options are disabled if you lack
image-export permission.
+### Editing Dashboard Tabs
Review Comment:
Dropped the section.
##########
superset-frontend/src/dashboard/components/gridComponents/Tabs/Tabs.tsx:
##########
@@ -166,6 +166,23 @@ const Tabs = (props: TabsProps): ReactElement => {
const prevTabIds = usePrevious(props.component.children);
useEffect(() => {
+ // Resolve missing or deleted active keys when children become available
+ // so a tab added to an empty container is selected and registered.
+ const tabId = props.component.children[selectedTabIndex];
+ if (!props.component.children.includes(activeKey) && tabId) {
+ setActiveKey(tabId);
+ }
+ }, [activeKey, props.component.children, selectedTabIndex]);
+
+ useEffect(() => {
+ // A TABS component with no children resolves no tab id, so there is
+ // nothing to activate. Dispatching the unresolved id would register an
+ // `undefined` entry in dashboardState.activeTabs, which JSON.stringify
+ // coerces to `null` when the dashboard state is posted to the permalink
+ // endpoint.
+ if (!activeKey) {
Review Comment:
Changed to `useState<string | undefined>`. That turned
`children.includes(activeKey)` into a type error, so the guard is now `tabId &&
(!activeKey || !children.includes(activeKey))` — same behaviour, and
`activeKey` narrows. `TabsRendererProps.activeKey` was the only consumer
declaring it `string`, so I widened that too; it forwards straight to antd's
`activeKey`, which already accepts `undefined`.
--
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]
