codeant-ai-for-open-source[bot] commented on code in PR #37502:
URL: https://github.com/apache/superset/pull/37502#discussion_r2737872907
##########
docs/src/theme/ReactLiveScope/index.tsx:
##########
@@ -18,36 +18,49 @@
*/
import React from 'react';
-import { Button, Card, Input, Space, Tag, Tooltip } from 'antd';
-// Import extension components from @apache-superset/core/ui
-// This matches the established pattern used throughout the Superset codebase
-// Resolved via webpack alias to
superset-frontend/packages/superset-core/src/ui/components
-import { Alert } from '@apache-superset/core/ui';
+// Browser-only check for SSR safety
+const isBrowser = typeof window !== 'undefined';
/**
* ReactLiveScope provides the scope for live code blocks.
* Any component added here will be available in ```tsx live blocks.
*
- * To add more components:
- * 1. Import the component from @apache-superset/core above
- * 2. Add it to the scope object below
+ * Components are conditionally loaded only in the browser to avoid
+ * SSG issues with Emotion CSS-in-JS jsx runtime.
+ *
+ * Components are available by name, e.g.:
+ * <Button>Click me</Button>
+ * <Avatar size="large" />
+ * <Badge count={5} />
*/
-const ReactLiveScope = {
+
+// Base scope with React (always available)
+const ReactLiveScope: Record<string, unknown> = {
// React core
React,
...React,
+};
- // Extension components from @apache-superset/core
- Alert,
+// Only load Superset components in browser context
+// This prevents SSG errors from Emotion CSS-in-JS
+if (isBrowser) {
+ try {
+ // Dynamic require for browser-only execution
+ // eslint-disable-next-line @typescript-eslint/no-require-imports
+ const SupersetComponents = require('@superset/components');
+ // eslint-disable-next-line @typescript-eslint/no-require-imports
+ const { Alert } = require('@apache-superset/core/ui');
- // Common Ant Design components (for demos)
- Button,
- Card,
- Input,
- Space,
- Tag,
- Tooltip,
-};
+ console.log('[ReactLiveScope] SupersetComponents keys:',
Object.keys(SupersetComponents || {}).slice(0, 10));
+ console.log('[ReactLiveScope] Has Button?', 'Button' in
(SupersetComponents || {}));
+
+ Object.assign(ReactLiveScope, SupersetComponents, { Alert });
Review Comment:
**Suggestion:** Only the `Alert` component is pulled from
`@apache-superset/core/ui` into the live scope, so any `tsx live` examples that
reference other core UI components (like Button, Avatar, Badge, etc.) will
resolve to `undefined` and cause runtime "invalid element type" errors; merging
the full core UI namespace into `ReactLiveScope` matches the pattern used in
`StorybookWrapper` and ensures all UI components are available. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Live TSX examples with core UI components fail to render.
- ⚠️ Developer Portal documentation UX broken for affected components.
- ⚠️ Examples referencing core-ui show runtime React errors.
```
</details>
```suggestion
const CoreUI = require('@apache-superset/core/ui');
Object.assign(ReactLiveScope, SupersetComponents, CoreUI);
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open a docs page containing a live TSX example that references a core UI
component
other than Alert (for example, a <Button /> example). Live examples use
ReactLiveScope
from docs/src/theme/ReactLiveScope/index.tsx to resolve component names.
2. With the current PR code, the require for '@superset/components' is
merged into the
scope, but from '@apache-superset/core/ui' only Alert is pulled and merged
(see lines
around docs/src/theme/ReactLiveScope/index.tsx:51-58). Other core UI exports
(Button,
Avatar, etc.) are not added to ReactLiveScope.
3. When the live example renderer (the docs live/tsx executor) tries to
render <Button />
it will look up Button in ReactLiveScope and find undefined, resulting in
runtime "Invalid
element type" or similar React rendering error in the browser console and
broken live
example UI.
4. Reproduce by loading a docs page with a live example that uses a core-ui
component
besides Alert; the page will show errors and the example will fail to
render. This is a
direct consequence of only merging Alert instead of the full core UI
namespace into
ReactLiveScope.
5. Fixing by merging the full core UI namespace (Object.assign(..., CoreUI))
or
individually exposing other needed components resolves the runtime failures
for those live
examples.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** docs/src/theme/ReactLiveScope/index.tsx
**Line:** 55:58
**Comment:**
*Logic Error: Only the `Alert` component is pulled from
`@apache-superset/core/ui` into the live scope, so any `tsx live` examples that
reference other core UI components (like Button, Avatar, Badge, etc.) will
resolve to `undefined` and cause runtime "invalid element type" errors; merging
the full core UI namespace into `ReactLiveScope` matches the pattern used in
`StorybookWrapper` and ensures all UI components are available.
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.
```
</details>
##########
docs/src/webpack.extend.ts:
##########
@@ -26,12 +27,73 @@ export default function webpackExtendPlugin(): Plugin<void>
{
configureWebpack(config) {
const isDev = process.env.NODE_ENV === 'development';
+ // Use NormalModuleReplacementPlugin to forcefully replace react-table
+ // This is necessary because regular aliases don't work for modules in
nested node_modules
+ const reactTableShim = path.resolve(__dirname, './shims/react-table.js');
+ config.plugins?.push(
+ new webpack.NormalModuleReplacementPlugin(
+ /^react-table$/,
+ reactTableShim,
+ ),
+ );
+
+ // Stub out heavy third-party packages that are transitive dependencies
of
+ // superset-frontend components. The barrel file (components/index.ts)
+ // re-exports all components, so webpack must resolve their imports even
+ // though these components are never rendered on the docs site.
+ const nullModuleShim = path.resolve(__dirname, './shims/null-module.js');
+ const heavyDepsPatterns = [
+ /^brace(\/|$)/, // ACE editor modes/themes
+ /^react-ace(\/|$)/,
+ /^ace-builds(\/|$)/,
+ /^react-js-cron(\/|$)/, // Cron picker + CSS
+ // react-resize-detector: NOT shimmed — DropdownContainer needs it at
runtime
+ // for overflow detection. Resolves from
superset-frontend/node_modules.
+ /^react-window(\/|$)/,
+ /^re-resizable(\/|$)/,
+ /^react-draggable(\/|$)/,
+ /^ag-grid-react(\/|$)/,
+ /^ag-grid-community(\/|$)/,
+ ];
+ heavyDepsPatterns.forEach(pattern => {
+ config.plugins?.push(
+ new webpack.NormalModuleReplacementPlugin(pattern, nullModuleShim),
+ );
+ });
+
// Add YAML loader rule directly to existing rules
config.module?.rules?.push({
test: /\.ya?ml$/,
use: 'js-yaml-loader',
});
+ // Add babel-loader rule for superset-frontend files
+ // This ensures Emotion CSS-in-JS is processed correctly for SSG
+ const supersetFrontendPath = path.resolve(
+ __dirname,
+ '../../superset-frontend',
+ );
+ config.module?.rules?.push({
Review Comment:
**Suggestion:** Adding loader rules with `config.module?.rules?.push`
silently does nothing if `config.module` or `config.module.rules` is missing,
so the YAML loader and `babel-loader` for `superset-frontend` may never be
registered and required files will not be processed, causing build or runtime
issues. [possible bug]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Build errors parsing YAML files.
- ❌ Untranspiled superset-frontend TSX breaks.
- ⚠️ Emotion styles missing in SSG output.
```
</details>
```suggestion
// Ensure module rules array exists before pushing new rules
if (!config.module) {
config.module = {};
}
if (!config.module.rules) {
config.module.rules = [];
}
// Add YAML loader rule directly to existing rules
config.module.rules.push({
test: /\.ya?ml$/,
use: 'js-yaml-loader',
});
// Add babel-loader rule for superset-frontend files
// This ensures Emotion CSS-in-JS is processed correctly for SSG
const supersetFrontendPath = path.resolve(
__dirname,
'../../superset-frontend',
);
config.module.rules.push({
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run the docs site build which invokes the configureWebpack function in
docs/src/webpack.extend.ts. The file contains two pushes into
config.module?.rules at
lines 64-69 (YAML loader) and 70-95 (babel-loader for superset-frontend).
2. If the incoming webpack config has no module or no module.rules array, the
optional-chained pushes (config.module?.rules?.push(...)) are no-ops and the
YAML loader
and babel-loader rules are never registered.
3. Without the YAML loader, .yml/.yaml docs or story metadata will not be
parsed during
the build and may cause missing content or loader errors. Without the
babel-loader rule
targeting the superset-frontend path, Emotion's automatic runtime and
TypeScript/JSX in
superset-frontend files won't be transpiled for the static site generator,
causing runtime
CSS omissions or syntax errors during build when those files are imported.
4. Reproduce by running the docs build with a webpack config variant that
omits
module.rules (or start with a fresh Docusaurus config that doesn't provide
rules). Observe
that loaders are not applied and the build emits errors related to YAML
parsing or
untranspiled TSX syntax.
Note: Making the module and rules arrays explicit (ensure they exist) before
pushing makes
the behavior deterministic and prevents silent no-ops.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** docs/src/webpack.extend.ts
**Line:** 64:76
**Comment:**
*Possible Bug: Adding loader rules with `config.module?.rules?.push`
silently does nothing if `config.module` or `config.module.rules` is missing,
so the YAML loader and `babel-loader` for `superset-frontend` may never be
registered and required files will not be processed, causing build or runtime
issues.
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.
```
</details>
##########
docs/src/theme/Playground/Preview/index.tsx:
##########
@@ -0,0 +1,107 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import React, { type ReactNode } from 'react';
+import { LiveError, LivePreview } from 'react-live';
+import BrowserOnly from '@docusaurus/BrowserOnly';
+import { ErrorBoundaryErrorMessageFallback } from '@docusaurus/theme-common';
+import ErrorBoundary from '@docusaurus/ErrorBoundary';
+import Translate from '@docusaurus/Translate';
+import PlaygroundHeader from '@theme/Playground/Header';
+
+import styles from './styles.module.css';
+
+// Get the theme wrapper for Superset components
+function getThemeWrapper() {
+ if (typeof window === 'undefined') {
+ return ({ children }: { children: React.ReactNode }) => <>{children}</>;
+ }
+
+ try {
+ // eslint-disable-next-line @typescript-eslint/no-require-imports
+ const { themeObject } = require('@apache-superset/core/ui');
+ // eslint-disable-next-line @typescript-eslint/no-require-imports
+ const { App } = require('antd');
+
+ if (!themeObject?.SupersetThemeProvider) {
Review Comment:
**Suggestion:** The code assumes that `antd` exports an `App` component; if
the installed Ant Design version does not provide this export, `App` will be
`undefined`, and rendering `<App>{children}</App>` will throw a runtime
"element type is invalid" error inside the live playground instead of cleanly
falling back. Guarding on `App`'s presence (similar to the existing
`SupersetThemeProvider` check) avoids this hard failure and transparently falls
back to an unwrapped preview when `App` is unavailable. [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Live playgrounds crash on pages with React live previews.
- ⚠️ Developer Portal interactive examples unavailable.
- ⚠️ Error surfaces in browser console during docs dev.
```
</details>
```suggestion
if (!themeObject?.SupersetThemeProvider || !App) {
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the file docs/src/theme/Playground/Preview/index.tsx and locate
getThemeWrapper()
(file lines ~31-55 in PR). The require/import block for
'@apache-superset/core/ui' and
'antd' is at lines 36-50 in the PR diff.
2. Run the documentation site (Docusaurus dev server) that uses this
PlaygroundPreview
component so LivePreview is mounted (start dev server and visit any page
containing a live
code playground which renders PlaygroundPreview -> component at
docs/src/theme/Playground/Preview/index.tsx:91).
3. Use an environment where the installed Ant Design package does not export
an App symbol
(e.g., ant-design version without App export). In that case require('antd')
returns an
object where App is undefined (observed at require call in index.tsx lines
37-38).
4. getThemeWrapper() proceeds because themeObject?.SupersetThemeProvider
exists (checked
at index.tsx line ~42) and returns a wrapper that renders
<App>{children}</App> (index.tsx
lines 46-50). React will throw "Element type is invalid" at render time when
attempting to
render undefined as a component; the live preview then fails with a runtime
React error
inside the Playground page instead of cleanly falling back.
5. Confirm reproduction by checking browser console and Docusaurus page:
error stack will
point to the PlaygroundPreview/ThemedLivePreview render path and the failing
component
originates from docs/src/theme/Playground/Preview/index.tsx lines 46-50.
Note: The proposed change simply guards on App presence (similar to the
existing
SupersetThemeProvider check) so that when App is undefined the wrapper falls
back to
returning children unwrapped; the current code renders <App> unconditionally
when
themeObject exists, causing the runtime render error in the described
environment.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** docs/src/theme/Playground/Preview/index.tsx
**Line:** 41:42
**Comment:**
*Possible Bug: The code assumes that `antd` exports an `App` component;
if the installed Ant Design version does not provide this export, `App` will be
`undefined`, and rendering `<App>{children}</App>` will throw a runtime
"element type is invalid" error inside the live playground instead of cleanly
falling back. Guarding on `App`'s presence (similar to the existing
`SupersetThemeProvider` check) avoids this hard failure and transparently falls
back to an unwrapped preview when `App` is unavailable.
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.
```
</details>
##########
docs/src/webpack.extend.ts:
##########
@@ -26,12 +27,73 @@ export default function webpackExtendPlugin(): Plugin<void>
{
configureWebpack(config) {
const isDev = process.env.NODE_ENV === 'development';
+ // Use NormalModuleReplacementPlugin to forcefully replace react-table
+ // This is necessary because regular aliases don't work for modules in
nested node_modules
+ const reactTableShim = path.resolve(__dirname, './shims/react-table.js');
+ config.plugins?.push(
+ new webpack.NormalModuleReplacementPlugin(
+ /^react-table$/,
+ reactTableShim,
+ ),
+ );
+
+ // Stub out heavy third-party packages that are transitive dependencies
of
+ // superset-frontend components. The barrel file (components/index.ts)
+ // re-exports all components, so webpack must resolve their imports even
+ // though these components are never rendered on the docs site.
+ const nullModuleShim = path.resolve(__dirname, './shims/null-module.js');
+ const heavyDepsPatterns = [
+ /^brace(\/|$)/, // ACE editor modes/themes
+ /^react-ace(\/|$)/,
+ /^ace-builds(\/|$)/,
+ /^react-js-cron(\/|$)/, // Cron picker + CSS
+ // react-resize-detector: NOT shimmed — DropdownContainer needs it at
runtime
+ // for overflow detection. Resolves from
superset-frontend/node_modules.
+ /^react-window(\/|$)/,
+ /^re-resizable(\/|$)/,
+ /^react-draggable(\/|$)/,
+ /^ag-grid-react(\/|$)/,
+ /^ag-grid-community(\/|$)/,
+ ];
+ heavyDepsPatterns.forEach(pattern => {
+ config.plugins?.push(
Review Comment:
**Suggestion:** Using optional chaining when pushing to `config.plugins`
means that if the base webpack config doesn't define a `plugins` array, your
NormalModuleReplacementPlugin registrations are silently skipped, leaving heavy
dependencies and `react-table` unshimmed and causing resolution/build failures
when those modules are imported. [possible bug]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Docs site build can fail resolving react-table.
- ❌ Heavy deps get bundled unexpectedly.
- ⚠️ SSG CSS-in-JS behavior may break silently.
```
</details>
```suggestion
if (!config.plugins) {
config.plugins = [];
}
config.plugins.push(
new webpack.NormalModuleReplacementPlugin(
/^react-table$/,
reactTableShim,
),
);
// Stub out heavy third-party packages that are transitive
dependencies of
// superset-frontend components. The barrel file (components/index.ts)
// re-exports all components, so webpack must resolve their imports
even
// though these components are never rendered on the docs site.
const nullModuleShim = path.resolve(__dirname,
'./shims/null-module.js');
const heavyDepsPatterns = [
/^brace(\/|$)/, // ACE editor modes/themes
/^react-ace(\/|$)/,
/^ace-builds(\/|$)/,
/^react-js-cron(\/|$)/, // Cron picker + CSS
// react-resize-detector: NOT shimmed — DropdownContainer needs it
at runtime
// for overflow detection. Resolves from
superset-frontend/node_modules.
/^react-window(\/|$)/,
/^re-resizable(\/|$)/,
/^react-draggable(\/|$)/,
/^ag-grid-react(\/|$)/,
/^ag-grid-community(\/|$)/,
];
heavyDepsPatterns.forEach(pattern => {
config.plugins!.push(
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Build the docs site using the PR code by running the Docusaurus build
script (entry:
docs package.json build). The build will load the webpack extension at
docs/src/webpack.extend.ts which contains the plugin registration logic.
2. The code at docs/src/webpack.extend.ts lines 30-38 (react-table
replacement) and 44-62
(heavyDepsPatterns loop) uses optional chaining when pushing to
config.plugins:
config.plugins?.push(...). If the base webpack config passed into
configureWebpack lacks a
plugins array, these push calls are skipped (no plugin registrations happen).
3. During module resolution, a Superset component import of react-table (or
any heavy
dependency listed) will be resolved normally (no
NormalModuleReplacementPlugin was
registered). This leads to webpack attempting to resolve and bundle the real
modules,
causing build failures or huge bundles because the shims/null-module are not
used. The
failure manifests as module not found, incorrect CommonJS/ESM interop, or
excessive bundle
size during the docs build.
4. Verify by inspecting the final file docs/src/webpack.extend.ts and
reproducing with a
minimal environment where the incoming config.resolve/plugins is empty: run
the docs build
and observe that react-table shim and heavy-deps shim messages/plugins are
absent and
imports of those modules fail.
Note: The existing pattern of optional chaining is risky here because
configureWebpack
receives a third-party config shape that may validly omit plugins; the more
robust
behavior is to ensure config.plugins exists before pushing.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** docs/src/webpack.extend.ts
**Line:** 31:59
**Comment:**
*Possible Bug: Using optional chaining when pushing to `config.plugins`
means that if the base webpack config doesn't define a `plugins` array, your
NormalModuleReplacementPlugin registrations are silently skipped, leaving heavy
dependencies and `react-table` unshimmed and causing resolution/build failures
when those modules are imported.
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.
```
</details>
##########
superset-frontend/packages/superset-ui-core/src/components/AutoComplete/AutoComplete.stories.tsx:
##########
@@ -252,16 +261,60 @@ const AutoCompleteWithOptions = (args: AutoCompleteProps)
=> {
return <AutoComplete {...args} options={options} onSearch={handleSearch} />;
};
+
type Story = StoryObj<typeof AutoComplete>;
-export const AutoCompleteStory: Story = {
+// Interactive story with static options - works in both Storybook and Docs
+export const InteractiveAutoComplete: Story = {
+ args: {
+ style: { width: 300 },
+ placeholder: 'Type to search...',
+ options: staticOptions,
+ filterOption: true, // Enable built-in filtering for static options
+ },
+ argTypes: {
+ options: {
+ control: false,
+ description: 'The dropdown options',
+ },
+ filterOption: {
+ control: 'boolean',
+ description: 'Enable filtering of options based on input',
+ },
+ },
+};
+
+// Docs configuration - provides static options for documentation rendering
+InteractiveAutoComplete.parameters = {
+ docs: {
+ staticProps: {
+ options: [
+ { value: 'Dashboard', label: 'Dashboard' },
+ { value: 'Chart', label: 'Chart' },
+ { value: 'Dataset', label: 'Dataset' },
+ { value: 'Database', label: 'Database' },
+ { value: 'Query', label: 'Query' },
+ ],
Review Comment:
**Suggestion:** The options array for the docs `staticProps` is duplicated
instead of reusing the `staticOptions` constant; if one copy is updated and the
other is forgotten, the interactive story and the generated documentation will
show inconsistent option lists, so referencing the shared constant in
`staticProps` avoids this drift. [code quality]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Storybook interactive demo shows different options than docs.
- ⚠️ Generated MDX docs list stale option values.
- ⚠️ Contributors may update one list and forget the other.
```
</details>
```suggestion
options: staticOptions,
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the file
`superset-frontend/packages/superset-ui-core/src/components/AutoComplete/AutoComplete.stories.tsx`
and locate the docs staticProps block starting at line 288 (final file
state). Confirm the
inline options array exists at lines 291–297 (the duplicated list).
2. Edit the top-level `staticOptions` constant at lines ~220–227 (the shared
constant
defined earlier in the same file) to add, remove, or rename an option (for
example change
'Query' -> 'Queries').
3. Build or run Storybook/docs generation (the docs generator reads story
parameters.docs.staticProps). Observe that the interactive story
(InteractiveAutoComplete.args.options referencing `staticOptions` at lines
~268–274) shows
the updated `staticOptions` values in Storybook, but the auto-generated MDX
docs that
consume the docs `staticProps` block (the inline array at lines 291–297)
still show the
previous values.
4. Result: documentation and interactive story diverge because the inline
`staticProps.options` array (lines 291–297) was not updated with the shared
`staticOptions` constant; this is reproducible by comparing the two lists in
the same file
and verifying differing output in Storybook (interactive story) vs. docs
pages.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/packages/superset-ui-core/src/components/AutoComplete/AutoComplete.stories.tsx
**Line:** 291:297
**Comment:**
*Code Quality: The options array for the docs `staticProps` is
duplicated instead of reusing the `staticOptions` constant; if one copy is
updated and the other is forgotten, the interactive story and the generated
documentation will show inconsistent option lists, so referencing the shared
constant in `staticProps` avoids this drift.
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.
```
</details>
##########
superset-frontend/packages/superset-ui-core/src/components/MetadataBar/MetadataBar.stories.tsx:
##########
@@ -98,3 +106,85 @@ Basic.argTypes = {
},
},
};
+
+// Interactive story for docs generation
+export const InteractiveMetadataBar = (args: MetadataBarProps) => (
+ <MetadataBar {...args} />
+);
+
+InteractiveMetadataBar.args = {};
Review Comment:
**Suggestion:** The new interactive story does not provide the required
`items` prop to the underlying component, so when this story is rendered in
Storybook `MetadataBar` receives `items` as `undefined`, which leads to it
computing an empty items array and throwing the runtime error about the minimum
number of items; you should initialize `InteractiveMetadataBar.args` with a
valid `items` array (e.g., mirroring the existing basic example) so that the
story renders without error. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Storybook InteractiveMetadataBar story fails to render.
- ❌ Developer Portal docs generator may hit runtime error.
- ⚠️ Local development story-based documentation broken.
- ⚠️ CI storybook snapshot builds may fail.
```
</details>
```suggestion
InteractiveMetadataBar.args = {
items: [
{
type: MetadataType.Sql,
title: 'Click to view query',
},
{
type: MetadataType.Owner,
createdBy: 'Jane Smith',
owners: ['John Doe', 'Mary Wilson'],
createdOn: A_WEEK_AGO,
},
{
type: MetadataType.LastModified,
value: A_WEEK_AGO,
modifiedBy: 'Jane Smith',
},
{
type: MetadataType.Tags,
values: ['management', 'research', 'poc'],
},
{
type: MetadataType.Dashboards,
title: 'Added to 3 dashboards',
description: 'To preview the list of dashboards go to More settings.',
},
],
};
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open Storybook (or run the docs generator that renders stories) so
stories from this
file are mounted. The story definition is in
superset-frontend/packages/superset-ui-core/src/components/MetadataBar/MetadataBar.stories.tsx
(see file lines 110-119; the Interactive story is declared at lines 110-113
and its args
at line 115).
2. Story rendering passes the story args into the story component:
InteractiveMetadataBar
at line 111 spreads args into <MetadataBar {...args} /> (file:
MetadataBar.stories.tsx:111-113). Because InteractiveMetadataBar.args is an
empty object
(MetadataBar.stories.tsx:115), the args object contains no items property.
3. The MetadataBar component
(superset-frontend/packages/superset-ui-core/src/components/MetadataBar/MetadataBar.tsx)
receives items as undefined; the implementation expects an items array and
enforces
minimum/maximum counts. The component computes uniqueItems and then throws
if too few
items (see MetadataBar.tsx lines ~183-190 where it declares items:
ContentType[] and
throws "The minimum number of items for the metadata bar is 2." at
MetadataBar.tsx:190).
4. As a result, rendering the InteractiveMetadataBar story causes a runtime
Error to be
thrown and the story fails to render. Reproduced by running Storybook and
opening the
"Design System/Components/MetadataBar" Interactive story; the failure
corresponds to the
throw at
superset-frontend/packages/superset-ui-core/src/components/MetadataBar/MetadataBar.tsx:190.
5. Mitigation: initialize InteractiveMetadataBar.args with a valid items
array (mirror
Basic.args defined at MetadataBar.stories.tsx:71-99); this supplies the
required items
prop and prevents the thrown error during story render.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/packages/superset-ui-core/src/components/MetadataBar/MetadataBar.stories.tsx
**Line:** 115:115
**Comment:**
*Logic Error: The new interactive story does not provide the required
`items` prop to the underlying component, so when this story is rendered in
Storybook `MetadataBar` receives `items` as `undefined`, which leads to it
computing an empty items array and throwing the runtime error about the minimum
number of items; you should initialize `InteractiveMetadataBar.args` with a
valid `items` array (e.g., mirroring the existing basic example) so that the
story renders without error.
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.
```
</details>
##########
superset-frontend/packages/superset-ui-core/src/components/Grid/Grid.stories.tsx:
##########
@@ -26,18 +26,30 @@ export default {
title: 'Design System/Components/Grid',
component: Row,
subcomponents: { Col },
+ parameters: {
+ docs: {
+ description: {
+ component:
+ 'The Grid system of Ant Design is based on a 24-grid layout. The
`Row` and `Col` components are used to create flexible and responsive grid
layouts.',
+ },
+ },
+ },
+} as Meta<typeof Row>;
+
+type Story = StoryObj<typeof Row>;
+
+export const InteractiveGrid: Story = {
+ args: {
+ align: 'top',
+ justify: 'start',
+ wrap: true,
+ gutter: 16,
Review Comment:
**Suggestion:** The default `gutter` value exposed via story args (16) does
not match the actual initial gutter used in the story (24), so the generated
documentation and Storybook controls show a misleading default that doesn't
match the rendered layout; aligning these values avoids this inconsistency.
[logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Developer docs show incorrect default gutter.
- ⚠️ Storybook controls mismatch confuses component consumers.
```
</details>
```suggestion
gutter: 24,
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the story file at
superset-frontend/packages/superset-ui-core/src/components/Grid/Grid.stories.tsx
and
inspect the story args block at lines 42-47 which sets gutter: 16.
2. Run Storybook and open the "Design System/Components/Grid" ->
"InteractiveGrid" story.
The Storybook Controls panel reads the story args and will display a default
gutter value
of 16 (from the args at lines 42-47).
3. In the same file, inspect the InteractiveGrid render implementation where
the story
initializes internal state: `const [gutter, setGutter] = useState(24);` (the
useState line
is in the render block, near the render function beginning — see the
InteractiveGrid
render block where gutter is created). This causes the rendered layout to
start with
gutter = 24.
4. Observe the mismatch: Controls show default 16 but the rendered component
uses 24 on
first render. Toggle the gutter control in Storybook — the control updates
the internal
state only when changed, but initial visual layout differs from the
control's shown
default.
Explanation: The mismatch is reproducible and deterministic because
args.gutter (16) and
the component's internal useState default (24) are both present in the final
file;
aligning them removes the inconsistency.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/packages/superset-ui-core/src/components/Grid/Grid.stories.tsx
**Line:** 46:46
**Comment:**
*Logic Error: The default `gutter` value exposed via story args (16)
does not match the actual initial gutter used in the story (24), so the
generated documentation and Storybook controls show a misleading default that
doesn't match the rendered layout; aligning these values avoids this
inconsistency.
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.
```
</details>
##########
superset-frontend/packages/superset-ui-core/src/components/IconButton/IconButton.stories.tsx:
##########
@@ -22,6 +22,28 @@ import { IconButton } from '.';
export default {
title: 'Components/IconButton',
component: IconButton,
+ parameters: {
+ docs: {
+ description: {
+ component:
+ 'The IconButton component is a versatile button that allows you to
combine an icon with a text label. It is designed for use in situations where
you want to display an icon along with some text in a single clickable
element.',
+ },
+ a11y: {
+ enabled: true,
+ },
Review Comment:
**Suggestion:** The `a11y` configuration is currently nested under
`parameters.docs`, but Storybook's a11y addon and similar tooling expect `a11y`
to be a top-level `parameters` entry; as written, the accessibility checks will
not be applied to this story as intended. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Storybook a11y checks not applied to IconButton story.
- ⚠️ Developer Portal docs may omit accessibility metadata.
- ⚠️ Local developer QA could miss accessibility regressions.
```
</details>
```suggestion
},
a11y: {
enabled: true,
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the story file at
superset-frontend/packages/superset-ui-core/src/components/IconButton/IconButton.stories.tsx
and inspect export default (lines 22-36). The file shows
`parameters.docs.a11y` defined at
lines 31-33.
2. Start Storybook (developer workflow) and load the Components/IconButton
story —
Storybook will read the story's top-level `parameters` for addons. Because
`a11y` is
nested under `parameters.docs` (lines 26-34) instead of being a top-level
`parameters.a11y`, the Storybook a11y addon will not receive that config for
the story.
3. Observe that the a11y panel or automated checks do not apply the intended
`enabled:
true` behavior for this story (absence of expected a11y results in Storybook
UI). The
relevant code location is the same story file lines 22-36 where `a11y` is
nested
incorrectly.
4. Confirm fix by moving `a11y: { enabled: true }` to `parameters.a11y` (as
in
improved_code). Restart Storybook and verify the a11y addon activates for
Components/IconButton story.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/packages/superset-ui-core/src/components/IconButton/IconButton.stories.tsx
**Line:** 31:33
**Comment:**
*Logic Error: The `a11y` configuration is currently nested under
`parameters.docs`, but Storybook's a11y addon and similar tooling expect `a11y`
to be a top-level `parameters` entry; as written, the accessibility checks will
not be applied to this story as intended.
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.
```
</details>
##########
superset-frontend/packages/superset-ui-core/src/components/EditableTitle/EditableTitle.stories.tsx:
##########
@@ -37,10 +37,71 @@ InteractiveEditableTitle.args = {
title: 'Title',
defaultTitle: 'Default title',
placeholder: 'Placeholder',
+ certifiedBy: '',
+ certificationDetails: '',
maxWidth: 100,
autoSize: true,
};
InteractiveEditableTitle.argTypes = {
+ canEdit: {
+ description: 'Whether the title can be edited.',
+ },
+ editing: {
+ description: 'Whether the title is currently in edit mode.',
+ },
+ emptyText: {
+ description: 'Text to display when title is empty.',
+ },
+ noPermitTooltip: {
+ description: 'Tooltip shown when user lacks edit permission.',
+ },
+ showTooltip: {
+ description: 'Whether to show tooltip on hover.',
+ },
+ title: {
+ description: 'The title text to display.',
+ },
+ defaultTitle: {
+ description: 'Default title when none is provided.',
+ },
+ placeholder: {
+ description: 'Placeholder text when editing.',
+ },
+ certifiedBy: {
+ description: 'Name of person/team who certified this item.',
+ },
+ certificationDetails: {
+ description: 'Additional certification details or description.',
+ },
+ maxWidth: {
+ description: 'Maximum width of the title in pixels.',
+ },
+ autoSize: {
+ description: 'Whether to auto-size based on content.',
+ },
onSaveTitle: { action: 'onSaveTitle' },
};
+
+InteractiveEditableTitle.parameters = {
+ actions: {
+ disable: true,
+ },
Review Comment:
**Suggestion:** Disabling Storybook actions at the story level while relying
on `argTypes.onSaveTitle` to wire an action handler means `onSaveTitle` will no
longer be auto-injected, leaving a required callback prop undefined; when the
component calls this function on blur, Storybook will throw a runtime "is not a
function" error. Removing the `actions.disable` override restores the automatic
action stub while keeping the docs configuration intact. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Storybook interactive Demo may throw runtime error.
- ⚠️ Live example "Try it" broken for EditableTitle story.
- ⚠️ Maintainers lose automatic action logging in Storybook UI.
```
</details>
```suggestion
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open Storybook and navigate to the EditableTitle story defined in
superset-frontend/packages/superset-ui-core/src/components/EditableTitle/EditableTitle.stories.tsx
(story exported as InteractiveEditableTitle). The parameters block for this
story is
defined starting at line 86 in the PR hunk.
2. Note that InteractiveEditableTitle.argTypes already defines onSaveTitle
as an action
(see line ~83: onSaveTitle: { action: 'onSaveTitle' }). However the story
parameters
(lines 86-107) set actions.disable = true which disables Storybook's
automatic action
wiring for the story.
3. Interact with the rendered story in Storybook UI (for example, focus and
blur the title
editor). The EditableTitle component (imported at top of the same file) will
call the
provided onSaveTitle prop during the blur/save flow. Because actions are
disabled for the
story, Storybook will not inject the action stub for onSaveTitle and the
prop may be
undefined, causing "is not a function" or similar runtime error when the
component invokes
it.
4. Observe the runtime error in the browser console and broken interactive
behavior in
Storybook. Reproducing requires loading this exact story file (path above)
with the
parameters block present; removing actions.disable or providing an explicit
onSaveTitle
prop in the story args prevents the error.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/packages/superset-ui-core/src/components/EditableTitle/EditableTitle.stories.tsx
**Line:** 87:89
**Comment:**
*Logic Error: Disabling Storybook actions at the story level while
relying on `argTypes.onSaveTitle` to wire an action handler means `onSaveTitle`
will no longer be auto-injected, leaving a required callback prop undefined;
when the component calls this function on blur, Storybook will throw a runtime
"is not a function" error. Removing the `actions.disable` override restores the
automatic action stub while keeping the docs configuration intact.
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.
```
</details>
##########
superset-frontend/packages/superset-ui-core/src/components/Divider/Divider.stories.tsx:
##########
@@ -30,6 +30,7 @@ InteractiveDivider.args = {
dashed: false,
variant: 'solid',
orientation: 'center',
+ orientationMargin: '',
plain: true,
Review Comment:
**Suggestion:** Setting `orientationMargin` to an empty string in the story
args forces the prop to be passed as `""`, overriding the Divider component's
own default margin behavior and causing the Developer Portal's live example to
no longer reflect the component's real default state; omitting it from `args`
lets the component manage its own default while still exposing a control via
`argTypes`. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Live example misrepresents component defaults.
- ⚠️ Developer Portal shows inaccurate usage.
- ⚠️ Storybook controls still available but start value wrong.
```
</details>
```suggestion
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open Developer Portal or Storybook for the Divider component (file:
superset-frontend/packages/superset-ui-core/src/components/Divider/Divider.stories.tsx).
The story function is defined at the top of that file and args are set
starting at line
30.
2. Observe the live example rendered by Storybook/Developer Portal uses
InteractiveDivider.args (lines 30-36). Because orientationMargin is set to
the empty
string on line 33, Storybook will pass orientationMargin="" to the Divider
component on
render.
3. The Divider component will receive an explicit empty-string prop rather
than no prop;
the component's internal default value (its own prop default) is therefore
overridden.
This makes the live example reflect orientationMargin="" instead of the
component's normal
default behavior.
4. Verify by interacting with the story controls: clearing or changing the
orientationMargin control will still show the empty-string baseline unless
the arg is
removed. Removing orientationMargin from the args object (as in the improved
code)
restores the component's internal default rendering in the Developer Portal.
Note: This path is based on the final file state where args are set in the
story (lines
30–36). The existing pattern directly passes arg values from
InteractiveDivider.args to
the rendered component.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/packages/superset-ui-core/src/components/Divider/Divider.stories.tsx
**Line:** 34:34
**Comment:**
*Logic Error: Setting `orientationMargin` to an empty string in the
story args forces the prop to be passed as `""`, overriding the Divider
component's own default margin behavior and causing the Developer Portal's live
example to no longer reflect the component's real default state; omitting it
from `args` lets the component manage its own default while still exposing a
control via `argTypes`.
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.
```
</details>
##########
superset-frontend/packages/superset-ui-core/src/components/FaveStar/FaveStar.stories.tsx:
##########
@@ -71,3 +71,44 @@ export const Default: Story = {
</div>
),
};
+
+export const InteractiveFaveStar: Story = {
+ args: {
+ itemId: 1,
+ isStarred: false,
+ showTooltip: true,
+ },
+ argTypes: {
+ isStarred: {
+ control: 'boolean',
+ description: 'Whether the item is currently starred.',
+ },
+ showTooltip: {
+ control: 'boolean',
+ description: 'Show tooltip on hover.',
+ },
+ },
+ render: args => (
+ <span style={{ display: 'inline-block' }}>
+ <FaveStar {...args} />
+ </span>
+ ),
+ parameters: {
+ docs: {
+ description: {
+ story: 'A star icon for marking items as favorites.',
+ },
+ liveExample: `function Demo() {
+ const [starred, setStarred] = React.useState(false);
+ return (
+ <FaveStar
+ itemId={1}
+ isStarred={starred}
+ showTooltip
+ fetchFaveStar={() => setStarred(!starred)}
Review Comment:
**Suggestion:** The `liveExample` uses an inline `fetchFaveStar={() =>
setStarred(!starred)}` callback, which FaveStar calls from a `useEffect` that
depends on `fetchFaveStar`, causing an infinite render loop and still not
wiring the required `saveFaveStar` toggle handler; the example should instead
update state via `saveFaveStar`. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Live demo may enter infinite update loop in Storybook.
- ⚠️ Developer Portal's Try It example shows incorrect API usage.
- ⚠️ Contributors may copy flawed example into production.
```
</details>
```suggestion
saveFaveStar={(_, isStarred) => setStarred(!isStarred)}
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the generated docs/Storybook page that shows the liveExample
embedded from this
story:
- File:
superset-frontend/packages/superset-ui-core/src/components/FaveStar/FaveStar.stories.tsx
- liveExample string is defined at lines 101-111 in the PR hunk.
2. The liveExample Demo mounts and passes fetchFaveStar={() =>
setStarred(!starred)} into
FaveStar (see line 107 in the snippet).
3. If the FaveStar component internally uses fetchFaveStar inside a
useEffect() that
includes fetchFaveStar in its dependency array (a common pattern for
effectful callbacks),
the inline arrow created on every render will change identity and retrigger
the effect,
causing setStarred to toggle repeatedly and producing an infinite
render/update loop.
4. Reproducing: load the liveExample in Storybook and observe repeated state
toggles or
console warnings about too many updates; replacing the prop with a stable
handler (e.g.,
saveFaveStar that flips state based on the passed value) or memoizing the
callback stops
the loop.
5. Note: The story's liveExample also appears to wire the wrong semantic
callback —
passing a "fetch" prop where a "save/toggle" handler is expected — so the
example both
risks repeated renders and doesn't reflect the intended API for toggling
saved state. The
liveExample is embedded as a string in the story parameters (lines 101-111),
so updating
that code string to use a stable saveFaveStar implementation prevents the
behavior.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/packages/superset-ui-core/src/components/FaveStar/FaveStar.stories.tsx
**Line:** 108:108
**Comment:**
*Logic Error: The `liveExample` uses an inline `fetchFaveStar={() =>
setStarred(!starred)}` callback, which FaveStar calls from a `useEffect` that
depends on `fetchFaveStar`, causing an infinite render loop and still not
wiring the required `saveFaveStar` toggle handler; the example should instead
update state via `saveFaveStar`.
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.
```
</details>
##########
superset-frontend/packages/superset-ui-core/src/components/FaveStar/FaveStar.stories.tsx:
##########
@@ -71,3 +71,44 @@ export const Default: Story = {
</div>
),
};
+
+export const InteractiveFaveStar: Story = {
+ args: {
+ itemId: 1,
+ isStarred: false,
+ showTooltip: true,
+ },
Review Comment:
**Suggestion:** The component requires a `saveFaveStar` callback, but the
interactive story's `args` omit it, so clicking the star in Storybook will
result in a runtime error (`saveFaveStar` is undefined/not a function) instead
of a safe no-op. [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Storybook interactive demo may throw runtime error on click.
- ⚠️ Developer Portal live demos could break for this component.
- ⚠️ New contributors see failing examples in docs.
```
</details>
```suggestion
saveFaveStar: (id: number, isStarred: boolean) => {
// default no-op; override in consumers or docs examples to handle
persistence
},
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open Storybook and navigate to the FaveStar component story in this file:
- File:
superset-frontend/packages/superset-ui-core/src/components/FaveStar/FaveStar.stories.tsx
- Story: InteractiveFaveStar (definition begins at the args block shown
at lines 76-80
in the PR hunk).
2. With the InteractiveFaveStar story selected, interact with the star
control (click the
star) in Storybook's canvas.
3. Because the story's args (lines 76-80) do not provide any
saveFaveStar/save handler
prop, if the FaveStar implementation calls a required callback like
saveFaveStar on click,
Storybook will attempt to call undefined and produce a runtime error in the
browser
console (TypeError: saveFaveStar is not a function).
4. Expected safe behavior (no error) would be to provide a default no-op
handler in the
story args so toggling the star in the interactive story does not crash the
demo. The
current code shows only itemId/isStarred/showTooltip at lines 76-80 and
therefore lacks
such a default handler.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/packages/superset-ui-core/src/components/FaveStar/FaveStar.stories.tsx
**Line:** 80:80
**Comment:**
*Possible Bug: The component requires a `saveFaveStar` callback, but
the interactive story's `args` omit it, so clicking the star in Storybook will
result in a runtime error (`saveFaveStar` is undefined/not a function) instead
of a safe no-op.
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.
```
</details>
##########
superset-frontend/packages/superset-ui-core/src/components/DatePicker/DatePicker.stories.tsx:
##########
@@ -80,11 +80,51 @@ export const InteractiveDatePicker: any = (args:
DatePickerProps) => (
InteractiveDatePicker.args = {
...commonArgs,
placeholder: 'Placeholder',
Review Comment:
**Suggestion:** The placeholder text in the story args (`'Placeholder'`)
does not match the placeholder in `parameters.docs.staticProps` (`'Select
date'`), so Storybook and the generated Developer Portal docs will show
different default placeholders for the same interactive example, breaking the
"stories as single source of truth" expectation and likely confusing users.
[logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Inconsistent placeholder between Storybook and Developer Portal.
- ⚠️ Confuses consumers reading component docs.
- ⚠️ Undermines "stories as single source" guarantee.
```
</details>
```suggestion
placeholder: 'Select date',
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the Storybook story definition in the repository file
superset-frontend/packages/superset-ui-core/src/components/DatePicker/DatePicker.stories.tsx
and locate InteractiveDatePicker.args (file lines around line 80). The args
currently
include "placeholder: 'Placeholder'" at line 82.
2. Build or run the Storybook site (normal developer flow). The interactive
example for
"Components/DatePicker" will render the DatePicker with the placeholder
value taken from
InteractiveDatePicker.args (the live canvas shows "Placeholder").
3. Run the docs generator described in the PR
(docs/scripts/generate-superset-components.mjs) which reads
story.parameters.docs.staticProps to produce Developer Portal MDX. The
generated MDX uses
staticProps.placeholder = 'Select date' (seen in the story parameters block
at lines
~103-115 in the same file).
4. Open the generated Developer Portal MDX or the docs view. You will
observe the
DatePicker example or its props table showing placeholder 'Select date'
while the
Storybook live example shows 'Placeholder' — a direct mismatch between
Storybook
interactive example (InteractiveDatePicker.args at line 82) and the
generator-sourced docs
(parameters.docs.staticProps at lines ~103-115).
5. Expected: both Storybook interactive example and generated MDX should
present the same
default placeholder. Reproducing this requires the existing PR file and
running Storybook
and the generator; the mismatch is deterministic from the two literal values
present in
the same file.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/packages/superset-ui-core/src/components/DatePicker/DatePicker.stories.tsx
**Line:** 82:82
**Comment:**
*Logic Error: The placeholder text in the story args (`'Placeholder'`)
does not match the placeholder in `parameters.docs.staticProps` (`'Select
date'`), so Storybook and the generated Developer Portal docs will show
different default placeholders for the same interactive example, breaking the
"stories as single source of truth" expectation and likely confusing users.
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.
```
</details>
##########
superset-frontend/packages/superset-ui-core/src/components/Divider/Divider.stories.tsx:
##########
@@ -38,16 +39,56 @@ InteractiveDivider.argTypes = {
variant: {
control: { type: 'select' },
options: ['dashed', 'dotted', 'solid'],
+ description: 'Line style of the divider.',
},
orientation: {
control: { type: 'select' },
options: ['left', 'right', 'center'],
+ description: 'Position of title inside divider.',
},
orientationMargin: {
control: { type: 'text' },
Review Comment:
**Suggestion:** The control for the margin prop is configured as a text
input even though the live example and the underlying Divider API expect a
numeric offset, so users typing bare numbers (like `50`) will produce string
values that can result in invalid CSS lengths and unexpected layout behavior;
using a number control keeps the values numeric and consistent with the
example. [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ LiveExample spacing breaks with string margin.
- ⚠️ Storybook controls produce inconsistent types.
- ⚠️ Documentation examples mismatch real usage.
```
</details>
```suggestion
control: { type: 'number' },
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the Divider story in Storybook/Developer Portal (file:
superset-frontend/packages/superset-ui-core/src/components/Divider/Divider.stories.tsx).
The argTypes are declared starting at line 39; orientationMargin is defined
at lines
49–52.
2. In the story controls UI, enter a bare number such as 50 into the
orientationMargin
text control (the control type is 'text' on line 50). Storybook will pass
this value as a
string ("50") to the Divider component.
3. The liveExample (embedded in
InteractiveDivider.parameters.docs.liveExample at lines
~75–92) demonstrates numeric offsets (e.g., orientationMargin={50}). When
the component
receives "50" (string) instead of numeric 50, the Divider implementation may
treat it as
an invalid CSS length or perform different runtime coercion, producing
incorrect spacing
or layout.
4. Changing the control type to 'number' (as in the improved_code) causes
Storybook to
pass a numeric value (number 50) consistently, matching the code examples in
the
liveExample and preventing type/coercion layout mismatches.
Note: This reproduction traces the control declaration (lines 49–52) to the
live example
usage shown in the same file's parameters (lines ~75–92) in the final file
state.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/packages/superset-ui-core/src/components/Divider/Divider.stories.tsx
**Line:** 50:50
**Comment:**
*Possible Bug: The control for the margin prop is configured as a text
input even though the live example and the underlying Divider API expect a
numeric offset, so users typing bare numbers (like `50`) will produce string
values that can result in invalid CSS lengths and unexpected layout behavior;
using a number control keeps the values numeric and consistent with the example.
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.
```
</details>
--
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]
