-
Notifications
You must be signed in to change notification settings - Fork 180
fix(Sender): sender 组件增加 focus、blur 回调事件 #350
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
WalkthroughAdded focus and blur event emissions in Sender, wired to the input. Implemented a click guard to prevent input focusing when interacting with the action list. Updated type definitions to include new emits. Storybook story now logs focus/blur events via @storybook/addon-actions. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant S as Sender Component
participant I as Input
participant A as Action List (.el-sender-action-list)
participant P as Parent (listener)
U->>I: Click / Focus
I-->>S: focus event
S-->>P: emit('focus', FocusEvent)
U->>A: Click action item
A-->>S: mousedown detected
Note over S: Stop propagation<br/>Blur input<br/>Return early (no focus)
S-->>I: blur()
I-->>S: blur event
S-->>P: emit('blur', FocusEvent)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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: 1
🧹 Nitpick comments (3)
packages/core/src/components/Sender/index.vue (1)
59-89: Address linting violations for code consistency.The static analysis detected multiple formatting issues that should be addressed to maintain code consistency:
- Missing newlines after if statements (antfu/if-newline rule)
- Closing curly braces on same line as subsequent blocks (style/brace-style rule)
While these are formatting issues rather than functional problems, addressing them improves code readability and maintains consistency with the project's linting standards.
Also applies to: 101-120, 134-149, 178-180, 185-186, 196-227, 260-265, 272-303, 324-326
packages/core/src/components/Sender/types.d.ts (2)
59-60: Nit: rename payload parameter for clarity (avoid “internalValue” for events).Use an event‑semantic name to avoid confusion with string payloads used by submit/cancel.
Apply this diff:
- (event: 'focus', internalValue: FocusEvent): void; - (event: 'blur', internalValue: FocusEvent): void; + (event: 'focus', evt: FocusEvent): void; + (event: 'blur', evt: FocusEvent): void;
59-60: Expose focus/blur methods in the public instance type.Issue #279 also asks for programmatic focus. Ensure the component exposes focus()/blur() and type them for refs so TS users get intellisense.
You can add an exposed interface in this d.ts and wire it in the SFC via defineExpose:
// In types.d.ts (same file) export interface SenderExpose { focus: () => void; blur: () => void; }And in packages/core/src/components/Sender/index.vue:
// inside <script setup lang="ts"> function focus() { /* ... focus the input ... */ } function blur() { /* ... blur the input ... */ } defineExpose<SenderExpose>({ focus, blur })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/core/src/components/Sender/index.vue(14 hunks)packages/core/src/components/Sender/types.d.ts(1 hunks)packages/core/src/stories/Sender/index.vue(3 hunks)
🧰 Additional context used
🪛 ESLint
packages/core/src/components/Sender/index.vue
[error] 59-59: Expect newline after if
(antfu/if-newline)
[error] 89-89: Expect newline after if
(antfu/if-newline)
[error] 101-101: Expect newline after if
(antfu/if-newline)
[error] 120-120: Closing curly brace appears on the same line as the subsequent block.
(style/brace-style)
[error] 134-134: Closing curly brace appears on the same line as the subsequent block.
(style/brace-style)
[error] 149-149: Closing curly brace appears on the same line as the subsequent block.
(style/brace-style)
[error] 178-178: Expect newline after if
(antfu/if-newline)
[error] 180-180: Expect newline after if
(antfu/if-newline)
[error] 185-185: Expect newline after if
(antfu/if-newline)
[error] 186-186: Expect newline after if
(antfu/if-newline)
[error] 196-196: Expect newline after if
(antfu/if-newline)
[error] 227-227: Closing curly brace appears on the same line as the subsequent block.
(style/brace-style)
[error] 260-260: Expect newline after if
(antfu/if-newline)
[error] 265-265: Expect newline after if
(antfu/if-newline)
[error] 272-272: Expect newline after if
(antfu/if-newline)
[error] 303-303: Closing curly brace appears on the same line as the subsequent block.
(style/brace-style)
[error] 324-324: Closing curly brace appears on the same line as the subsequent block.
(style/brace-style)
[error] 326-326: Closing curly brace appears on the same line as the subsequent block.
(style/brace-style)
🔇 Additional comments (7)
packages/core/src/stories/Sender/index.vue (3)
3-3: LGTM! Appropriate import for Storybook action logging.The Storybook action addon import is correctly added to support event logging in the development environment.
23-29: LGTM! Focus and blur handlers correctly implemented for Storybook.The handlers properly use the Storybook action addon for logging focus and blur events with descriptive Chinese labels. The FocusEvent typing is correct.
38-39: LGTM! Event handlers correctly bound to Sender component.The focus and blur event handlers are properly wired to the Sender component, enabling Storybook to log these events for development and testing purposes.
packages/core/src/components/Sender/index.vue (3)
164-170: LGTM! Smart click handling prevents unintended focus.The click guard logic correctly prevents the input from receiving focus when users interact with action buttons in the right panel. The implementation uses
contains()to detect clicks within the action list and appropriately stops propagation and blurs the input.
376-384: LGTM! Focus and blur event handlers correctly implemented.The handlers properly emit the focus and blur events with the native FocusEvent payload, allowing parent components to respond to these events as specified in the PR objectives.
466-467: LGTM! Event handlers correctly bound to input.The focus and blur events are properly wired to the input element, enabling the emission of these events to parent components.
packages/core/src/components/Sender/types.d.ts (1)
59-60: Emitting focus/blur looks good and aligns with #279.Adding typed 'focus' and 'blur' emits is consistent with existing event typing and supports the PR goal of letting callers react to focus changes.
| (event: 'focus', internalValue: FocusEvent): void; | ||
| (event: 'blur', internalValue: FocusEvent): void; |
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.
🧩 Analysis chain
Verify TS DOM lib availability and SFC wiring.
FocusEvent requires the DOM lib; also confirm the SFC actually emits and exposes as intended.
Run:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Check DOM lib presence in tsconfig..."
fd -a 'tsconfig*.json' | while read -r f; do
echo "==> $f"
rg -n '"lib"\s*:\s*\[[^]]*dom' -n "$f" || true
done
echo; echo "2) Confirm Sender emits include focus/blur..."
rg -nP -C2 '(defineEmits|emits\s*:)\s*' packages/core/src/components/Sender/index.vue || true
rg -nP "@focus=|@blur=" packages/core/src/components/Sender/index.vue || true
echo; echo "3) Confirm programmatic methods are exposed..."
rg -nP 'defineExpose\s*<' packages/core/src/components/Sender/index.vue || true
rg -nP '\bfunction\s+focus\b|\bfocus\s*\(\)\s*{' -n packages/core/src/components/Sender/index.vue || true
rg -nP '\bfunction\s+blur\b|\bblur\s*\(\)\s*{' -n packages/core/src/components/Sender/index.vue || trueLength of output: 450
Enable DOM lib in TS config & expose Sender focus/blur
- TS config: tsconfig files exist at apps/docs/tsconfig.json and packages/core/tsconfig.json but the scan found no explicit DOM lib — add "DOM" to packages/core/tsconfig.json compilerOptions.lib (or ensure the package inherits lib.dom) so FocusEvent types resolve.
- Sender SFC: defineEmits() is present (packages/core/src/components/Sender/index.vue:37) and the template listens to @focus/@blur (≈466–467); focus() and blur() functions exist (≈311–318) but no typed defineExpose was detected — expose the methods (e.g., defineExpose({ focus, blur }) or defineExpose(...)) and ensure SenderEmits includes the FocusEvent signatures.
🤖 Prompt for AI Agents
In packages/core/src/components/Sender/types.d.ts around lines 59-60, the
FocusEvent types and exposed focus/blur methods are not properly resolved/typed:
update packages/core/tsconfig.json to include "DOM" in compilerOptions.lib (or
ensure it inherits lib.dom) so FocusEvent is available, add the FocusEvent
signatures to the SenderEmits type so defineEmits<SenderEmits>() covers (event:
'focus', internalValue: FocusEvent): void and (event: 'blur', internalValue:
FocusEvent): void, and in packages/core/src/components/Sender/index.vue add a
typed defineExpose to expose the existing focus and blur functions (e.g.,
defineExpose({ focus, blur }) or defineExpose<ExposeType>(...)) so consumers and
the template have proper typings.
closed #279
Summary by CodeRabbit
New Features
Bug Fixes
Documentation