codeant-ai-for-open-source[bot] commented on code in PR #37502:
URL: https://github.com/apache/superset/pull/37502#discussion_r2739137680
##########
superset-frontend/packages/superset-ui-core/src/components/Menu/Menu.stories.tsx:
##########
@@ -47,18 +55,97 @@ export const InteractiveMenu = (args: any) => (
);
InteractiveMenu.args = {
- defaultSelectedKeys: ['1'],
- inlineCollapsed: false,
mode: 'horizontal',
- multiple: false,
selectable: true,
};
InteractiveMenu.argTypes = {
mode: {
- control: {
- type: 'select',
- },
+ control: 'select',
options: ['horizontal', 'vertical', 'inline'],
+ description:
+ 'Menu display mode: horizontal navbar, vertical sidebar, or inline
collapsible.',
+ },
+ selectable: {
+ control: 'boolean',
+ description: 'Whether menu items can be selected.',
+ },
+ multiple: {
+ control: 'boolean',
+ description: 'Allow multiple items to be selected.',
+ },
+ inlineCollapsed: {
+ control: 'boolean',
+ description:
+ 'Whether the inline menu is collapsed (only applies to inline mode).',
+ },
+};
+
+InteractiveMenu.parameters = {
+ docs: {
+ staticProps: {
+ items: [
+ { label: 'Dashboards', key: 'dashboards' },
+ { label: 'Charts', key: 'charts' },
+ { label: 'Datasets', key: 'datasets' },
+ { label: 'SQL Lab', key: 'sqllab' },
+ ],
+ },
+ liveExample: `function Demo() {
+ return (
+ <Menu
+ mode="horizontal"
+ selectable
+ items={[
+ { label: 'Dashboards', key: 'dashboards' },
+ { label: 'Charts', key: 'charts' },
+ { label: 'Datasets', key: 'datasets' },
+ { label: 'SQL Lab', key: 'sqllab' },
+ ]}
+ />
+ );
+}`,
+ examples: [
+ {
+ title: 'Vertical Menu',
+ code: `function VerticalMenu() {
+ return (
+ <Menu
+ mode="vertical"
+ style={{ width: 200 }}
+ items={[
+ { label: 'Dashboards', key: 'dashboards' },
+ { label: 'Charts', key: 'charts' },
+ { label: 'Datasets', key: 'datasets' },
+ {
+ label: 'Settings',
+ key: 'settings',
+ children: [
+ { label: 'Profile', key: 'profile' },
+ { label: 'Preferences', key: 'preferences' },
+ ],
+ },
+ ]}
+ />
+ );
+}`,
+ },
+ {
+ title: 'Menu with Icons',
+ code: `function MenuWithIcons() {
+ return (
Review Comment:
**Suggestion:** The "Menu with Icons" live example code uses `Icons.*` JSX
elements without importing `Icons`, which will cause a runtime reference error
(`Icons` is not defined) when the generated MDX `tsx live` snippet is executed
in the Developer Portal playground. [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Live playground throws ReferenceError, demo fails to render.
- ⚠️ Developer Portal "Try it" unusable for this example.
- ⚠️ Documentation page shows broken example preview.
```
</details>
```suggestion
code: `import { Icons } from '@superset-ui/core/components/Icons';
function MenuWithIcons() {
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open Developer Portal page generated from Storybook stories for Menu
(stories come from
superset-frontend/packages/superset-ui-core/src/components/Menu/Menu.stories.tsx).
The
"Menu with Icons" example is defined in this file at the code block starting
at the hunk
around lines 134-147 in the PR.
2. The generator emits a tsx live snippet for the example into an MDX page
(generator
noted in PR description: docs/scripts/generate-superset-components.mjs). The
generated
live snippet contains the example code exactly as found in the story: it
references
Icons.DashboardOutlined etc. but does not include any import for Icons.
3. Load the Developer Portal playground and run the live example. The
browser executes the
snippet; since Icons is not defined in the snippet scope, the console throws
a
ReferenceError: "Icons is not defined" and the live preview fails to render.
4. Confirm reproduction by inspecting the emitted MDX/tsx live block (copy
of the example
code) and verifying absence of an Icons import. Fix is to include an import
(for example
from the project's Icons export) in the example code so the playground has
the symbol
defined.
```
</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/Menu/Menu.stories.tsx
**Line:** 136:136
**Comment:**
*Possible Bug: The "Menu with Icons" live example code uses `Icons.*`
JSX elements without importing `Icons`, which will cause a runtime reference
error (`Icons` is not defined) when the generated MDX `tsx live` snippet is
executed in the Developer Portal playground.
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/Popover/Popover.stories.tsx:
##########
@@ -70,24 +53,116 @@ InteractivePopover.args = {
};
InteractivePopover.argTypes = {
+ content: {
+ control: 'text',
+ description: 'Content displayed inside the popover body.',
+ },
+ title: {
+ control: 'text',
+ description: 'Title displayed in the popover header.',
+ },
placement: {
- name: PLACEMENTS.label,
control: { type: 'select' },
- options: PLACEMENTS.options,
+ options: [
+ 'topLeft',
+ 'top',
+ 'topRight',
+ 'leftTop',
+ 'left',
+ 'leftBottom',
+ 'rightTop',
+ 'right',
+ 'rightBottom',
+ 'bottomLeft',
+ 'bottom',
+ 'bottomRight',
+ ],
+ description: 'Position of the popover relative to the trigger element.',
},
trigger: {
- name: TRIGGERS.label,
control: { type: 'select' },
- options: TRIGGERS.options,
+ options: ['hover', 'click', 'focus'],
+ description: 'Event that triggers the popover to appear.',
},
arrow: {
- name: 'arrow',
control: { type: 'boolean' },
- description: "Change arrow's visible state",
+ description: "Whether to show the popover's arrow pointing to the
trigger.",
},
color: {
- name: 'color',
control: { type: 'color' },
description: 'The background color of the popover.',
},
};
+
+InteractivePopover.parameters = {
+ docs: {
+ sampleChildren: [{ component: 'Button', props: { children: 'Hover me' } }],
+ liveExample: `function Demo() {
+ return (
+ <Popover
+ content="Popover sample content"
+ title="Popover title"
+ arrow
+ >
+ <Button>Hover me</Button>
+ </Popover>
+ );
+}`,
+ examples: [
+ {
+ title: 'Click Trigger',
+ code: `function ClickPopover() {
+ return (
+ <Popover
+ content="This popover appears on click."
+ title="Click Popover"
+ trigger="click"
+ >
+ <Button>Click me</Button>
+ </Popover>
+ );
+}`,
+ },
+ {
+ title: 'Placements',
+ code: `function PlacementsDemo() {
+ return (
+ <div style={{ display: 'flex', gap: 16, flexWrap: 'wrap', justifyContent:
'center', padding: '60px 0' }}>
+ {['top', 'right', 'bottom', 'left'].map(placement => (
+ <Popover
+ key={placement}
+ content={\`This popover is placed on the \${placement}\`}
+ title={placement}
+ placement={placement}
+ >
+ <Button>{placement}</Button>
+ </Popover>
+ ))}
+ </div>
+ );
+}`,
+ },
+ {
+ title: 'Rich Content',
+ code: `function RichPopover() {
+ return (
Review Comment:
**Suggestion:** The rich content example in
`InteractivePopover.parameters.docs.examples` uses `<Icons.InfoCircleOutlined
/>` inside a `tsx live` code snippet, but the corresponding generated MDX
(`docs/developer_portal/components/ui/popover.mdx`) does not import `Icons`, so
unless `Icons` is injected into the React Live scope globally, this example
will throw a `ReferenceError: Icons is not defined` when executed in the
Developer Portal. [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Developer Portal live example fails rendering.
- ⚠️ Popover docs Try It example shows runtime error.
- ⚠️ Confuses contributors editing examples in MDX.
```
</details>
```suggestion
code: `import { Icons } from '@apache-superset/core/ui';
function RichPopover() {
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the file
`superset-frontend/packages/superset-ui-core/src/components/Popover/Popover.stories.tsx`
and locate the Rich Content example at lines 146-165 (the "Rich Content"
entry under
InteractivePopover.parameters.docs.examples). The embedded code string
contains
`<Icons.InfoCircleOutlined />` but no import for `Icons`.
2. Run the generator described in the PR
(docs/scripts/generate-superset-components.mjs)
to produce MDX pages for component docs (the PR summary states this
generator emits `live`
code blocks derived from `parameters.docs.examples`). The generator will
copy the `code`
string into the generated MDX live snippet for the Popover page.
3. Serve the Developer Portal (or open the generated MDX in a React Live
environment).
When the Dev Portal tries to evaluate the live snippet, the runtime will
attempt to
resolve `Icons`. Because the snippet does not include `import { Icons } ...`
and the Dev
Portal only provides a limited React Live scope, the example will throw
ReferenceError:
Icons is not defined at evaluation time.
4. Observe the failure: the live preview for the "Rich Content" example on
the Popover
documentation page will not render and will show a runtime error in the
console/preview
area. This reproduces directly from the code at lines 146-165 of the story
file where the
example string is declared.
Note: This is a real issue only if the Developer Portal does not inject
`Icons` into the
React Live scope globally. If the portal already injects `Icons` globally
(verify in docs
build scope configuration), then the example will succeed; otherwise the
lack of an import
is the root cause.
```
</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/Popover/Popover.stories.tsx
**Line:** 148:148
**Comment:**
*Possible Bug: The rich content example in
`InteractivePopover.parameters.docs.examples` uses `<Icons.InfoCircleOutlined
/>` inside a `tsx live` code snippet, but the corresponding generated MDX
(`docs/developer_portal/components/ui/popover.mdx`) does not import `Icons`, so
unless `Icons` is injected into the React Live scope globally, this example
will throw a `ReferenceError: Icons is not defined` when executed in the
Developer Portal.
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/Switch/Switch.stories.tsx:
##########
@@ -39,15 +47,121 @@ InteractiveSwitch.args = {
checked: defaultCheckedValue,
disabled: false,
loading: false,
- title: 'Switch',
+ title: 'Toggle feature',
defaultChecked: defaultCheckedValue,
Review Comment:
**Suggestion:** The story passes both `checked` and `defaultChecked` to the
switch while also controlling `checked` via `useArgs`, which makes the
component simultaneously controlled and "defaulted"; in React this pattern can
trigger controlled/uncontrolled warnings and will cause the `defaultChecked`
control (if exposed) to have no effect, leading to confusing behavior in
Storybook and in the generated Developer Portal docs. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Storybook "Switch" live example shows confusing controls.
- ⚠️ Developer Portal docs show no-op defaultChecked example.
- ⚠️ React console may emit controlled/uncontrolled warnings.
- ⚠️ Docs consumers may assume defaultChecked works.
```
</details>
```suggestion
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open Storybook with the PR code (story file at
superset-frontend/packages/superset-ui-core/src/components/Switch/Switch.stories.tsx).
2. Navigate to the "Components/Switch" story (export default title at file
line 22-24).
3. The story component uses useArgs to control args: see useArgs import and
usage at lines
19 and 35-41. The rendered Switch receives a controlled prop at line 39:
checked={checked}.
4. The story args include both checked and defaultChecked
(InteractiveSwitch.args block at
lines 46-52). Because the rendered Switch is passed checked (controlled) the
defaultChecked entry is effectively ignored at render time.
5. In Storybook UI change the "defaultChecked" control — observed behavior:
toggling the
defaultChecked control does not change the rendered Switch (because checked
is
controlled), demonstrating the confusing/no-op control.
6. Additionally, run Storybook in the browser console: React may emit a
controlled/uncontrolled warning when a component flips between having
checked undefined
and defined across updates; the presence of both props in arg definitions
increases risk
of such warnings during arg updates because Storybook control updates can
transiently set
or remove args. Evidence of controlled prop flow in file: updateArgs call at
line 40
updates checked arg on user interaction, enforcing controlled behavior.
7. Conclusion: keeping defaultChecked in the args while the story renders a
controlled
checked prop causes confusing Storybook behavior (no-op control) and risks
React
controlled/uncontrolled warnings. Removing defaultChecked from
InteractiveSwitch.args (as
in improved_code) aligns story controls with the actual controlled render
path.
```
</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/Switch/Switch.stories.tsx
**Line:** 51:51
**Comment:**
*Logic Error: The story passes both `checked` and `defaultChecked` to
the switch while also controlling `checked` via `useArgs`, which makes the
component simultaneously controlled and "defaulted"; in React this pattern can
trigger controlled/uncontrolled warnings and will cause the `defaultChecked`
control (if exposed) to have no effect, leading to confusing behavior in
Storybook and in the generated Developer Portal docs.
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/Card/Card.stories.tsx:
##########
@@ -22,29 +22,94 @@ import type { CardProps } from './types';
export default {
title: 'Components/Card',
component: Card,
+ parameters: {
+ docs: {
+ description: {
+ component:
+ 'A container component for grouping related content. ' +
+ 'Supports titles, borders, loading states, and hover effects.',
+ },
+ },
+ },
};
export const InteractiveCard = (args: CardProps) => <Card {...args} />;
InteractiveCard.args = {
padded: true,
- title: 'Components/Card',
- children: 'Card content',
+ title: 'Dashboard Overview',
+ children:
+ 'This card displays a summary of your dashboard metrics and recent
activity.',
Review Comment:
**Suggestion:** The `children` default arg is split across two lines, but
the docs generator's `parseArgsContent` only parses simple `key: value` pairs
on a single line; as a result it fails to detect the default `children` value,
so the generated MDX Live Example renders a card with empty content and the
props table omits the default children. Keep the key and string literal on the
same line so the generator can correctly pick it up. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Generated docs show empty Card live example.
- ❌ Props table omits default children value.
- ⚠️ Developer Portal documentation less useful.
- ⚠️ Docs generator parsing limitation; intentional design tradeoff.
```
</details>
```suggestion
children: 'This card displays a summary of your dashboard metrics and
recent activity.',
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the story file at
superset-frontend/packages/superset-ui-core/src/components/Card/Card.stories.tsx
and
inspect the InteractiveCard default args at lines 36-46; note the children
arg is split
across two lines (lines 41-42).
2. Run the docs generator (docs/scripts/generate-superset-components.mjs)
which parses
story files. The generator's arg parser expects simple "key: value" pairs on
a single
line; when it reads the story source it tokenizes lines and fails to match a
multi-line
"children:" value (generator behavior described in PR summary).
3. The generator therefore does not detect the default children string for
InteractiveCard
and omits it from the generated MDX props table and liveExample code block.
The produced
Live Example renders <Card> with empty content.
4. Confirm by comparing the generated MDX for the Card component (output
under docs portal
generation output dir) — the props table will show no default for children
and the Try It
/ live example will render an empty card body.
5. Fix: keep the key and string literal on a single line (replace lines
41-42 with the
single-line form). Re-run the generator and verify generated MDX contains
the children
default and the liveExample renders the expected text inside the Card.
```
</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/Card/Card.stories.tsx
**Line:** 41:42
**Comment:**
*Logic Error: The `children` default arg is split across two lines, but
the docs generator's `parseArgsContent` only parses simple `key: value` pairs
on a single line; as a result it fails to detect the default `children` value,
so the generated MDX Live Example renders a card with empty content and the
props table omits the default children. Keep the key and string literal on the
same line so the generator can correctly pick it up.
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/Badge/Badge.stories.tsx:
##########
@@ -59,55 +58,119 @@ const SIZES = {
defaultValue: undefined,
};
-export const InteractiveBadge = (args: BadgeProps) => <Badge {...args} />;
+// Count Badge - shows a number
+export const InteractiveBadge = (args: BadgeProps) => (
+ <Badge {...args}>
+ <div
+ style={{
+ width: 40,
+ height: 40,
+ background: '#eee',
+ borderRadius: 4,
+ }}
+ />
+ </Badge>
+);
InteractiveBadge.args = {
- count: undefined,
- color: undefined,
- text: 'Text',
- status: 'success',
+ count: 5,
size: 'default',
showZero: false,
overflowCount: 99,
};
InteractiveBadge.argTypes = {
- status: {
- control: {
- type: 'select',
- },
- options: [undefined, ...STATUSES],
- description:
- 'only works if `count` is `undefined` (or is set to 0) and `color` is
set to `undefined`',
+ count: {
+ description: 'Number to show in the badge.',
+ control: { type: 'number' },
},
size: {
- control: {
- type: 'select',
- },
- options: SIZES.options,
+ description: 'Size of the badge.',
+ control: { type: 'select' },
+ options: ['default', 'small'],
},
color: {
- control: {
- type: 'select',
- },
- options: [undefined, ...COLORS.options],
- },
- count: {
- control: {
- type: 'select',
- defaultValue: undefined,
- },
- options: [undefined, ...Array(100).keys()],
- defaultValue: undefined,
+ description: 'Custom background color for the badge.',
+ control: { type: 'select' },
+ options: [
+ 'pink',
+ 'red',
+ 'yellow',
+ 'orange',
+ 'cyan',
+ 'green',
+ 'blue',
+ 'purple',
+ 'geekblue',
+ 'magenta',
+ 'volcano',
+ 'gold',
+ 'lime',
+ ],
},
showZero: {
- control: 'boolean',
- defaultValue: false,
+ description: 'Whether to show badge when count is zero.',
+ control: { type: 'boolean' },
},
overflowCount: {
- control: 'number',
- description:
- 'The threshold at which the number overflows with a `+` e.g if you set
this to 10, and the value is 11, you get `11+`',
+ description: 'Max count to show. Shows count+ when exceeded (e.g., 99+).',
+ control: { type: 'number' },
+ },
+};
+
+InteractiveBadge.parameters = {
+ docs: {
+ description: {
+ story: 'Badge can show a count number or a status indicator dot.',
+ },
+ examples: [
+ {
+ title: 'Status Badge',
+ code: `function StatusBadgeDemo() {
+ const statuses = ['default', 'success', 'processing', 'warning', 'error'];
+ return (
+ <div style={{ display: 'flex', flexDirection: 'column', gap: 12 }}>
+ {statuses.map(status => (
+ <Badge key={status} status={status} text={\`Status: \${status}\`} />
Review Comment:
**Suggestion:** The example code string for the status badge demo uses
escaped template literals (`\`...\`` and `\${...}`), which the MDX generator
reads as raw text and then emits directly into a `tsx live` block; this
produces invalid TSX (`text={\`Status: \${status}\`}`) that will fail to parse
when the generated docs are built. Changing the snippet to use a plain string
expression avoids the need for escaping and yields valid code in the generated
MDX. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Developer Portal MDX build fails parsing generated Badge page.
- ❌ CI documentation pipeline breaks on generated MDX files.
- ⚠️ Consumers cannot view interactive Badge examples.
- ⚠️ Docs generator emits invalid TSX from stories.
```
</details>
```suggestion
<Badge key={status} status={status} text={'Status: ' + status} />
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the story file at
superset-frontend/packages/superset-ui-core/src/components/Badge/Badge.stories.tsx
and
locate the InteractiveBadge.parameters examples block (see lines ~126-139 in
the PR hunk).
The example currently contains the code string with escaped template usage:
`text={\`Status: \${status}\`}`.
2. Run the documentation generator used by this PR (generator:
docs/scripts/generate-superset-components.mjs — described in the PR
summary). The
generator reads story.parameters.examples.code and emits that string
directly into a `tsx
live` code block in the generated MDX page for the Badge component.
3. The emitted MDX will contain the literal backslash-escaped template
sequence (e.g.,
`text={\`Status: \${status}\`}`) inside a tsx live block. The MDX/TSX parser
will attempt
to parse this as TypeScript/JSX and encounter a syntax error because the
backslashes
produce invalid TSX expression syntax.
4. When building the Developer Portal or rendering the generated MDX, the
build will fail
with a TSX parse/compile error referencing the generated MDX file (the error
originates
from the invalid `text={\`...\`}` expression produced from the story string).
Note: This reproduction is based on the actual final story file content
(Badge.stories.tsx) which contains the escaped template literal in the
examples block and
the documented behavior of the generator in the PR summary that it emits
examples.code
into `tsx live` blocks. The issue is not hypothetical — the generator emits
the string
verbatim which will be parsed by the MDX/TSX toolchain.
```
</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/Badge/Badge.stories.tsx
**Line:** 133:134
**Comment:**
*Logic Error: The example code string for the status badge demo uses
escaped template literals (`\`...\`` and `\${...}`), which the MDX generator
reads as raw text and then emits directly into a `tsx live` block; this
produces invalid TSX (`text={\`Status: \${status}\`}`) that will fail to parse
when the generated docs are built. Changing the snippet to use a plain string
expression avoids the need for escaping and yields valid code in the generated
MDX.
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]
