Skip to content

Conversation

@Chang-Chen
Copy link

@Chang-Chen Chang-Chen commented Sep 16, 2025

整合组件内的按钮组件,新增ELXButton,兼容目前的零散按钮,传 elxButtonType 进行区分

export const elxButtonTypes = [
'clear',
'loading',
'send',
'speech',
'speechLoading',
''
] as const;

image

Summary by CodeRabbit

  • New Features

    • Introduced ELXButton with preset types: clear, loading, send, speech, and speech-loading, featuring built-in icons/animations and type-specific events (clear, cancel, submit).
    • Globally available in TypeScript projects for template autocomplete without manual imports.
  • Style

    • Added styles for ELXButton variants, including visual tweaks for clear and loading states.
  • Documentation

    • Added Storybook demos showcasing basic usage, multiple variants, and slot customization (e.g., custom loading content).

@coderabbitai
Copy link

coderabbitai bot commented Sep 16, 2025

Walkthrough

Introduces a new ELXButton component with type-based variants, associated subcomponents, styles, Storybook stories, and TypeScript global component declarations. Exports ELXButton from the components barrel. No runtime changes outside the new components; updates primarily add presentational SFCs, props/emits, and Storybook demos.

Changes

Cohort / File(s) Summary
Global typings
packages/core/components.d.ts
Adds ELXButton and five subcomponents to Vue GlobalComponents for TypeScript template usage.
Barrel export
packages/core/src/components.ts
Re-exports ELXButton from ./components/ELXButton/index.vue.
ELXButton core API and component
packages/core/src/components/ELXButton/index.ts, packages/core/src/components/ELXButton/index.vue
Introduces ELXButton props/emits/types and useButton handler; SFC wraps ElButton, maps elxButtonType to default-slot content and emits type-specific events.
ELXButton subcomponents
packages/core/src/components/ELXButton/components/ELXClearButton.vue, .../ELXLoadingButton.vue, .../ELXSendButton.vue, .../ELXSpeechButton.vue, .../ELXSpeechLoadingButton.vue
Adds five presentational SFCs for clear/send/speech/loading states; one includes animated SVG with a className prop.
Styles
packages/core/src/components/ELXButton/style.scss
Adds .elx-button base and type modifiers, adjusts padding and icon/loader appearance for specific variants.
Storybook: meta and demos
packages/core/src/stories/ELXButton/ELXButton.stories.ts, packages/core/src/stories/ELXButton/index.vue, packages/core/src/stories/ELXButton/ELXButtonSlot.vue
Adds Storybook metadata and two demos: variant showcase and slot customization, plus a slot-focused story component.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor U as User
  participant SB as Storybook/App
  participant EB as ELXButton (index.vue)
  participant UB as useButton (index.ts)
  participant EP as ElButton
  participant SC as Subcomponent (type-mapped)

  U->>SB: Click UI
  SB->>EB: Render with props (elxButtonType, disabled, loading, ...)
  EB->>EP: Render ElButton with merged attrs/props
  EB->>EB: Select default slot via ELXButtonMap
  EB->>SC: Render type-specific content (if type)
  U-->>EP: click(MouseEvent)
  EP->>UB: onClick handler
  UB->>UB: if disabled or loading -> return
  UB-->>EB: emit 'clear' | 'cancel' | 'submit' | fallback 'click'
  EB-->>SB: Event bubbled to listeners
  note over UB,EB: Emission depends on elxButtonType
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • HeJiaYue520

Poem

Hop hop, a button new, I see—
Clear, send, and speech in harmony.
Loading lights go spinny-spin,
Slots that twirl, events begin.
In stories bright, I test and grin—
ELX clicks; let reviews begin! 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title accurately describes the PR’s primary change: adding a consolidated ELXButton and using elxButtonType to support existing scattered button implementations, which matches the added ELXButton component, its subcomponents, props, emits, types, styles, and stories in the changeset. It is specific to the main functional addition and does not introduce unrelated content. The presence of an ellipsis is minor and does not make the title misleading.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (17)
packages/core/src/components/ELXButton/components/ELXLoadingButton.vue (2)

5-12: Improve SVG accessibility (explicit labelling).

Expose the title via aria-labelledby so SRs announce it; otherwise mark decorative.

-  <svg
+  <svg
     class="loading-svg"
     viewBox="0 0 1000 1000"
     xmlns="http://www.w3.org/2000/svg"
-    xmlns:xlink="http://www.w3.org/1999/xlink"
+    role="img"
+    aria-labelledby="elx-loading-title"
   >
-    <title>Loading</title>
+    <title id="elx-loading-title">Loading</title>

9-9: Remove unused xmlns:xlink.

No xlink attributes used; drop the obsolete namespace.

packages/core/src/components/ELXButton/components/ELXClearButton.vue (1)

5-8: Mark icon decorative or provide an accessible name.

If the parent button has an accessible label, hide the icon from SRs.

-  <el-icon><Brush /></el-icon>
+  <el-icon aria-hidden="true"><Brush /></el-icon>
packages/core/src/components/ELXButton/components/ELXSpeechButton.vue (1)

5-8: Mark icon decorative or label appropriately.

Same rationale as ClearButton.

-  <el-icon><Microphone /></el-icon>
+  <el-icon aria-hidden="true"><Microphone /></el-icon>
packages/core/src/components/ELXButton/components/ELXSpeechLoadingButton.vue (3)

28-35: Add a11y labelling on SVG.

Expose the <title> via aria-labelledby or mark decorative.

-  <svg
+  <svg
     :class="className"
     color="currentColor"
     :viewBox="`0 0 ${SIZE} ${SIZE}`"
     xmlns="http://www.w3.org/2000/svg"
-    xmlns:xlink="http://www.w3.org/1999/xlink"
+    role="img"
+    aria-labelledby="elx-speech-loading-title"
   >
-    <title>Speech Recording</title>
+    <title id="elx-speech-loading-title">Speech Recording</title>

2-4: Vue API consistency: consider dropping className prop and rely on class fallthrough.

Root <svg> already receives class from $attrs; default the class in markup and let callers override/merge via class.

-withDefaults(defineProps<{ className?: string }>(), {
-  className: 'loading-svg'
-});
+defineProps<{}>();
-  <svg
-    :class="className"
+  <svg
+    class="loading-svg"

Also applies to: 29-31


27-35: SMIL used — add CSS fallback or gate for Safari

packages/core/src/components/ELXButton/components/ELXSpeechLoadingButton.vue lines 47–62 use two elements (height and y); SMIL is not reliably supported in Safari — replace with CSS keyframes (transform: scaleY; transform-origin: bottom) or gate with feature-detection. Would you like a CSS-based version that mimics the equalizer bars?

packages/core/src/components/ELXButton/components/ELXSendButton.vue (1)

5-8: Mark icon decorative or provide label.

Keep SR output clean if the button has its own label.

-  <el-icon><Top /></el-icon>
+  <el-icon aria-hidden="true"><Top /></el-icon>
packages/core/src/stories/ELXButton/index.vue (3)

6-10: Tighten logClick implementation.

Use the rest args and avoid mutating with splice; also check length on the local var.

-function logClick(...ags: any[]) {
-  if (arguments.length >= 2) {
-    action(ags[0])(...ags.splice(1));
-  }
-}
+function logClick(...args: any[]) {
+  if (args.length > 1) {
+    action(args[0])(...args.slice(1));
+  }
+}

2-2: Avoid .ts extension in type-only import (tooling quirks).

Safer to import '@components/ELXButton/index' to work across TS configs.


50-53: Prefer stable keys over index.

Use the button type as key for deterministic re-renders.

-      v-for="(item, index) in elxButtons"
-      :key="index"
+      v-for="item in elxButtons"
+      :key="item.elxButtonType"
packages/core/src/stories/ELXButton/ELXButtonSlot.vue (1)

5-9: Fix inconsistent arguments handling in logClick function

The function uses both ags parameter and arguments object, which creates confusion and potential bugs. The splice operation mutates the array unnecessarily.

Apply this diff to fix the implementation:

-function logClick(...ags: any[]) {
-  if (arguments.length >= 2) {
-    action(ags[0])(...ags.splice(1));
-  }
+function logClick(actionName: string, ...args: any[]) {
+  if (args.length > 0) {
+    action(actionName)(...args);
+  }
 }
packages/core/src/components/ELXButton/index.vue (3)

1-1: Remove or update the misleading comment

The comment "ClearButton 清理按钮" appears to be copied from another component and doesn't accurately describe this ELXButton wrapper component.

Apply this diff:

-<!-- ClearButton 清理按钮 -->
+<!-- ELXButton 统一按钮组件 -->

15-15: Fix typo in import statement

The component import has an incorrect name - should be ELXSpeechLoadingButton to match the component filename and usage pattern.

Apply this diff:

-import ELXSpeechButtonLoading from './components/ELXSpeechLoadingButton.vue';
+import ELXSpeechLoadingButton from './components/ELXSpeechLoadingButton.vue';

And update line 40:

-  speechLoading: ELXSpeechButtonLoading
+  speechLoading: ELXSpeechLoadingButton

23-25: Consider simplifying the empty check logic

The nested optional chaining and trimming could be simplified.

Apply this diff for better readability:

 const elxButtonTypeEmpty = computed(() => {
-  return props.elxButtonType?.trim()?.length > 0;
+  return !!(props.elxButtonType?.trim());
 });
packages/core/src/components/ELXButton/index.ts (1)

46-53: Fix code style issues with if-else blocks

The ESLint errors indicate brace style violations where closing braces appear on the same line as subsequent blocks.

Apply this diff to fix the style issues:

     if (props.elxButtonType?.trim()?.length) {
       if (props.elxButtonType === 'clear') {
         return emit('clear', evt);
-      } else if (['loading', 'speechLoading'].includes(props.elxButtonType)) {
+      }
+      else if (['loading', 'speechLoading'].includes(props.elxButtonType)) {
         return emit('cancel', evt);
-      } else if (props.elxButtonType === 'send') {
+      }
+      else if (props.elxButtonType === 'send') {
         return emit('submit', evt);
       }
     }
packages/core/src/stories/ELXButton/ELXButton.stories.ts (1)

1-4: Standardize story import paths (alias vs relative)

ELXButton.stories.ts imports ELXButtonSource from '@components/ELXButton/index.vue' while packages/core/src/stories/ELXButton/index.vue imports { ELXButton } from '../../components' — the stories folder contains a mix of @components and relative imports; choose one convention and make imports consistent (either convert ELXButton.stories.ts to the relative '../../components' form or update local .vue files to use the @components alias).

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e3ce64d and 9a5e908.

📒 Files selected for processing (13)
  • packages/core/components.d.ts (1 hunks)
  • packages/core/src/components.ts (1 hunks)
  • packages/core/src/components/ELXButton/components/ELXClearButton.vue (1 hunks)
  • packages/core/src/components/ELXButton/components/ELXLoadingButton.vue (1 hunks)
  • packages/core/src/components/ELXButton/components/ELXSendButton.vue (1 hunks)
  • packages/core/src/components/ELXButton/components/ELXSpeechButton.vue (1 hunks)
  • packages/core/src/components/ELXButton/components/ELXSpeechLoadingButton.vue (1 hunks)
  • packages/core/src/components/ELXButton/index.ts (1 hunks)
  • packages/core/src/components/ELXButton/index.vue (1 hunks)
  • packages/core/src/components/ELXButton/style.scss (1 hunks)
  • packages/core/src/stories/ELXButton/ELXButton.stories.ts (1 hunks)
  • packages/core/src/stories/ELXButton/ELXButtonSlot.vue (1 hunks)
  • packages/core/src/stories/ELXButton/index.vue (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
packages/core/src/stories/ELXButton/index.vue (3)
packages/core/src/stories/FilesCard/FilesCard.stories.ts (1)
  • args (162-170)
packages/core/src/stories/Attachments/Attachments.stories.ts (1)
  • args (78-88)
packages/core/src/stories/Welcome/Welcome.stories.ts (1)
  • setup (105-107)
packages/core/src/stories/ELXButton/ELXButton.stories.ts (4)
packages/core/src/stories/Welcome/Welcome.stories.ts (1)
  • setup (105-107)
packages/core/src/stories/FilesCard/FilesCard.stories.ts (1)
  • args (162-170)
packages/core/src/stories/Conversations/Conversations.stories.ts (2)
  • setup (267-269)
  • args (263-271)
packages/core/src/stories/XMarkdown/Markdown.stories.ts (1)
  • args (90-98)
packages/core/src/components.ts (1)
packages/core/.build/scripts/auto-export-all-components.ts (4)
  • generateAutoEntry (13-87)
  • c (38-38)
  • c (45-45)
  • c (52-52)
packages/core/src/components/ELXButton/index.vue (1)
apps/docs/scripts/vue-element-plus-x-resolver.ts (2)
  • tovUIResolver (3-35)
  • resolve (6-33)
packages/core/components.d.ts (2)
packages/core/.build/scripts/auto-export-all-components.ts (2)
  • generateAutoEntry (13-87)
  • c (52-52)
apps/docs/scripts/vue-element-plus-x-resolver.ts (2)
  • tovUIResolver (3-35)
  • resolve (6-33)
packages/core/src/components/ELXButton/components/ELXClearButton.vue (1)
apps/docs/scripts/vue-element-plus-x-resolver.ts (1)
  • tovUIResolver (3-35)
packages/core/src/stories/ELXButton/ELXButtonSlot.vue (4)
packages/core/src/stories/FilesCard/FilesCard.stories.ts (1)
  • args (162-170)
packages/core/src/stories/Attachments/Attachments.stories.ts (1)
  • args (78-88)
packages/core/src/stories/Prompts/Prompts.stories.ts (1)
  • setup (76-78)
packages/core/src/stories/EditorSender/EditorSender.stories.ts (2)
  • setup (106-110)
  • _args (105-116)
🪛 ESLint
packages/core/src/components/ELXButton/index.ts

[error] 48-48: Closing curly brace appears on the same line as the subsequent block.

(style/brace-style)


[error] 50-50: Closing curly brace appears on the same line as the subsequent block.

(style/brace-style)

🔇 Additional comments (9)
packages/core/src/components/ELXButton/style.scss (1)

1-32: LGTM.

Sane defaults; selectors align with variants. Minor note: consider height: auto on .loading-svg if any consumers constrain height.

packages/core/src/components.ts (1)

8-8: Public export addition looks good.

Matches auto-export pattern and enables barrel import.

packages/core/src/stories/ELXButton/index.vue (1)

61-69: Story layout OK.

Centered demo is fine; no blocking issues.

packages/core/src/stories/ELXButton/ELXButtonSlot.vue (1)

18-23: Good implementation!

The custom button section properly binds attributes and correctly handles the click event with proper event typing.

packages/core/src/stories/ELXButton/ELXButton.stories.ts (1)

25-37: Well-structured story implementation

The story follows the established pattern consistently with other stories in the codebase, using the attrs-based setup approach.

packages/core/components.d.ts (1)

50-55: Global type declarations properly added

The new ELXButton component family type declarations follow the existing pattern and maintain alphabetical ordering within the new additions.

packages/core/src/components/ELXButton/index.vue (1)

61-77: Clean component composition pattern

The use of h() function with dynamic component rendering is well implemented, properly spreading attrs, props, and slots.

packages/core/src/components/ELXButton/index.ts (2)

4-11: Well-designed type system

The readonly tuple for button types and proper TypeScript typing provides excellent type safety and developer experience.


36-60: Consider adding the 'speech' type handling

The handleClick function handles all button types except 'speech'. This might be intentional, but worth verifying.

The 'speech' type is defined in elxButtonTypes but not handled in the switch logic. Is this intentional, falling back to the default 'click' event?

Comment on lines +30 to +47
<circle
cx="500"
cy="500"
fill="none"
r="450"
stroke="currentColor"
stroke-width="100"
stroke-dasharray="600 9999999"
>
<animateTransform
attributeName="transform"
dur="1s"
from="0 500 500"
repeatCount="indefinite"
to="360 500 500"
type="rotate"
/>
</circle>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Replace SMIL with CSS animation for cross‑browser support (Safari).

SMIL <animateTransform> has spotty support (notably Safari). Use CSS keyframes to rotate the arc instead.

Apply:

-    <circle
-      cx="500"
-      cy="500"
-      fill="none"
-      r="450"
-      stroke="currentColor"
-      stroke-width="100"
-      stroke-dasharray="600 9999999"
-    >
-      <animateTransform
-        attributeName="transform"
-        dur="1s"
-        from="0 500 500"
-        repeatCount="indefinite"
-        to="360 500 500"
-        type="rotate"
-      />
-    </circle>
+    <g class="elx-loading-spin" transform-origin="500px 500px">
+      <circle
+        cx="500"
+        cy="500"
+        fill="none"
+        r="450"
+        stroke="currentColor"
+        stroke-width="100"
+        stroke-dasharray="600 9999999"
+      />
+    </g>

And add styles:

-<style scoped lang="less"></style>
+<style scoped lang="less">
+@keyframes elx-spin { to { transform: rotate(360deg); } }
+.elx-loading-spin { animation: elx-spin 1s linear infinite; }
+</style>

Also applies to: 51-51

Comment on lines +15 to +24
// 计算矩形的位置和高度范围
const rects = computed(() => {
const dest = (SIZE - RECT_WIDTH * COUNT) / (COUNT - 1);
return Array.from({ length: COUNT }).map((_, index) => {
const x = index * (dest + RECT_WIDTH);
const yMin = SIZE / 2 - RECT_HEIGHT_MIN / 2;
const yMax = SIZE / 2 - RECT_HEIGHT_MAX / 2;
return { x, yMin, yMax };
});
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Import computed (compile error).

computed is used but not imported.

-<script setup lang="ts">
+<script setup lang="ts">
+import { computed } from 'vue';
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// 计算矩形的位置和高度范围
const rects = computed(() => {
const dest = (SIZE - RECT_WIDTH * COUNT) / (COUNT - 1);
return Array.from({ length: COUNT }).map((_, index) => {
const x = index * (dest + RECT_WIDTH);
const yMin = SIZE / 2 - RECT_HEIGHT_MIN / 2;
const yMax = SIZE / 2 - RECT_HEIGHT_MAX / 2;
return { x, yMin, yMax };
});
});
<script setup lang="ts">
import { computed } from 'vue';
// 计算矩形的位置和高度范围
const rects = computed(() => {
const dest = (SIZE - RECT_WIDTH * COUNT) / (COUNT - 1);
return Array.from({ length: COUNT }).map((_, index) => {
const x = index * (dest + RECT_WIDTH);
const yMin = SIZE / 2 - RECT_HEIGHT_MIN / 2;
const yMax = SIZE / 2 - RECT_HEIGHT_MAX / 2;
return { x, yMin, yMax };
});
});
</script>
🤖 Prompt for AI Agents
In packages/core/src/components/ELXButton/components/ELXSpeechLoadingButton.vue
around lines 15 to 24, the computed() function is used to build rects but
computed is not imported leading to a compile error; add the computed import
from 'vue' to the component's script imports (e.g., import { computed } from
'vue') and ensure it is included alongside other Vue composition API imports so
the computed usage resolves.

Comment on lines +43 to +45
const handleDefaultSlot = computed(() => {
return h(ELXButtonMap[props.elxButtonType] ?? slots.default);
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Potential issue with slot rendering

The computed property returns a VNode from h(), but when slots.default exists, it's already a function that returns VNodes. This could cause rendering issues.

Apply this diff to fix the slot handling:

 const handleDefaultSlot = computed(() => {
-  return h(ELXButtonMap[props.elxButtonType] ?? slots.default);
+  const Component = ELXButtonMap[props.elxButtonType];
+  return Component ? h(Component) : slots.default?.();
 });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const handleDefaultSlot = computed(() => {
return h(ELXButtonMap[props.elxButtonType] ?? slots.default);
});
const handleDefaultSlot = computed(() => {
const Component = ELXButtonMap[props.elxButtonType];
return Component ? h(Component) : slots.default?.();
});
🤖 Prompt for AI Agents
In packages/core/src/components/ELXButton/index.vue around lines 43-45, the
computed currently wraps slots.default (a function) with h(), which is
incorrect; instead check if slots.default exists and call it (e.g. return
slots.default() or slots.default?.()) to get its VNodes, otherwise return
h(ELXButtonMap[props.elxButtonType]) to create the mapped component VNode;
update the computed to call the slot function when present and only use h() for
the component fallback.

Comment on lines +2 to +5
import type { ELXButtonProps } from '@components/ELXButton/index.ts';
import { action } from '@storybook/addon-actions';
import { ELXButton } from '../../components';

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Import ref (compile error in Story).

ref is used but not imported.

-<script setup lang="ts">
-import type { ELXButtonProps } from '@components/ELXButton/index.ts';
+<script setup lang="ts">
+import { ref } from 'vue';
+import type { ELXButtonProps } from '@components/ELXButton/index';
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import type { ELXButtonProps } from '@components/ELXButton/index.ts';
import { action } from '@storybook/addon-actions';
import { ELXButton } from '../../components';
<script setup lang="ts">
import { ref } from 'vue';
import type { ELXButtonProps } from '@components/ELXButton/index';
import { action } from '@storybook/addon-actions';
import { ELXButton } from '../../components';
🤖 Prompt for AI Agents
In packages/core/src/stories/ELXButton/index.vue around lines 2 to 5, the Story
uses Vue's ref but doesn't import it, causing a compile error; add an import for
ref from 'vue' (e.g., include "import { ref } from 'vue'" or add ref to an
existing named import from 'vue') so the ref identifier is defined and the story
compiles.

@IsDyh01 IsDyh01 self-requested a review September 16, 2025 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant