-
Notifications
You must be signed in to change notification settings - Fork 183
feat(新增ELXButton): 为整合组件内的按钮组件,新增ELXButton,兼容目前的零散按钮,传 elxButtonType … #354
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughIntroduces 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
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
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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. Comment |
There was a problem hiding this 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-labelledbyso 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 unusedxmlns: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>viaaria-labelledbyor 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 droppingclassNameprop and rely on class fallthrough.Root
<svg>already receivesclassfrom$attrs; default the class in markup and let callers override/merge viaclass.-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 Safaripackages/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: TightenlogClickimplementation.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.tsextension 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
keyfor 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 inlogClickfunctionThe function uses both
agsparameter andargumentsobject, 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 commentThe 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 statementThe component import has an incorrect name - should be
ELXSpeechLoadingButtonto 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 logicThe 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 blocksThe 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
📒 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: autoon.loading-svgif 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 implementationThe 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 addedThe 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 patternThe 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 systemThe readonly tuple for button types and proper TypeScript typing provides excellent type safety and developer experience.
36-60: Consider adding the 'speech' type handlingThe
handleClickfunction handles all button types except 'speech'. This might be intentional, but worth verifying.The 'speech' type is defined in
elxButtonTypesbut not handled in the switch logic. Is this intentional, falling back to the default 'click' event?
| <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> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
| // 计算矩形的位置和高度范围 | ||
| 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 }; | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
| // 计算矩形的位置和高度范围 | |
| 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.
| const handleDefaultSlot = computed(() => { | ||
| return h(ELXButtonMap[props.elxButtonType] ?? slots.default); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
| 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.
| import type { ELXButtonProps } from '@components/ELXButton/index.ts'; | ||
| import { action } from '@storybook/addon-actions'; | ||
| import { ELXButton } from '../../components'; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
| 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.
整合组件内的按钮组件,新增ELXButton,兼容目前的零散按钮,传 elxButtonType 进行区分
export const elxButtonTypes = [
'clear',
'loading',
'send',
'speech',
'speechLoading',
''
] as const;
Summary by CodeRabbit
New Features
Style
Documentation