-
Notifications
You must be signed in to change notification settings - Fork 178
fix(bubble):修复无内容时气泡高度问题 #373
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
- 在 Bubble 组件中添加 'no-content' 类名判断 - 当应用 content 为空时 'no-content' 样式 - 修改样式文件使无内容气泡高度为0 - 移除无内容气泡的内边距
WalkthroughIntroduces a no-content CSS class state in the Bubble component’s template and applies matching style rules. Adjusts class bindings to include the no-content flag when content is absent. Minor whitespace/formatting changes in template and SCSS, plus stricter zero padding/height for no-content cases. No functional logic changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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: 2
🧹 Nitpick comments (1)
packages/core/src/components/Bubble/index.vue (1)
183-188: The nested no-content class is redundant but harmless.The typewriter wrapper's
no-contentclass binding mirrors the outer div's condition (line 180). Since the CSS selector.el-bubble-content .no-contenttargets descendants, this duplication is unnecessary but doesn't cause issues. Consider removing it for simplicity, or keep it for explicit clarity.If you prefer to simplify, apply this diff:
<div v-if="!loading" - class="el-typewriter" - :class="{ - 'no-content': !content - }" + class="el-typewriter" >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/core/src/components/Bubble/index.vue(2 hunks)packages/core/src/components/Bubble/style.scss(1 hunks)
🧰 Additional context used
🪛 ESLint
packages/core/src/components/Bubble/index.vue
[error] 70-70: Closing curly brace appears on the same line as the subsequent block.
(style/brace-style)
[error] 72-72: Closing curly brace appears on the same line as the subsequent block.
(style/brace-style)
🔇 Additional comments (2)
packages/core/src/components/Bubble/style.scss (2)
75-78: LGTM!The multi-line formatting of the
min-heightcalculation improves readability without changing the computed value.
82-87: No-content does not hide the loading indicatorVerified: Bubble applies 'no-content' when !content and also applies 'el-bubble-content-loading' and renders .el-bubble-loading-wrap when loading; style.scss does not set overflow:hidden and the loading markup is conditionally rendered, so the loading dots remain visible (see packages/core/src/components/Bubble/index.vue and packages/core/src/components/Bubble/style.scss).
| } else if (typeof props.typing === 'boolean') { | ||
| return props.typing; | ||
| } | ||
| else { | ||
| } else { |
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.
Fix the brace-style violations.
ESLint is flagging brace-style issues where closing curly braces appear on the same line as subsequent blocks. This violates the project's configured style guide.
Apply this diff to fix the formatting:
if (typeof props.typing === 'undefined') {
return false;
- } else if (typeof props.typing === 'boolean') {
+ }
+ else if (typeof props.typing === 'boolean') {
return props.typing;
- } else {
+ }
+ else {
return Object.assign({}, DEFAULT_TYPING, props.typing);
}📝 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.
| } else if (typeof props.typing === 'boolean') { | |
| return props.typing; | |
| } | |
| else { | |
| } else { | |
| if (typeof props.typing === 'undefined') { | |
| return false; | |
| } | |
| else if (typeof props.typing === 'boolean') { | |
| return props.typing; | |
| } | |
| else { | |
| return Object.assign({}, DEFAULT_TYPING, props.typing); | |
| } |
🧰 Tools
🪛 ESLint
[error] 70-70: Closing curly brace appears on the same line as the subsequent block.
(style/brace-style)
[error] 72-72: Closing curly brace appears on the same line as the subsequent block.
(style/brace-style)
🤖 Prompt for AI Agents
In packages/core/src/components/Bubble/index.vue around lines 70 to 72, the
brace-style is violated because a closing curly brace is on the same line as the
following else/else-if; reformat the conditional blocks so each closing brace is
on its own line before the next block (e.g., end the previous block with a
standalone "}" line, then start "else if (...)" or "else" on the next line) and
apply the same correction to any similar brace placements in this file to
satisfy the project's ESLint brace-style rule.
| 'el-bubble-content-shadow': variant === 'shadow' && !noStyle, | ||
| 'no-content': !content |
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.
Critical: no-content class hides the loading indicator.
When loading is true and content is empty, both el-bubble-content-loading and no-content classes are applied simultaneously. The no-content styles (height: 0; padding: 0;) will hide the loading indicator, breaking the loading state UX.
Apply this diff to conditionally apply the no-content class only when not loading:
'el-bubble-content-filled': variant === 'filled' && !noStyle,
'el-bubble-content-borderless': variant === 'borderless' && !noStyle,
'el-bubble-content-outlined': variant === 'outlined' && !noStyle,
'el-bubble-content-shadow': variant === 'shadow' && !noStyle,
- 'no-content': !content
+ 'no-content': !content && !loading
}"📝 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.
| 'el-bubble-content-shadow': variant === 'shadow' && !noStyle, | |
| 'no-content': !content | |
| :class="{ | |
| 'el-bubble-content-filled': variant === 'filled' && !noStyle, | |
| 'el-bubble-content-borderless': variant === 'borderless' && !noStyle, | |
| 'el-bubble-content-outlined': variant === 'outlined' && !noStyle, | |
| 'el-bubble-content-shadow': variant === 'shadow' && !noStyle, | |
| 'no-content': !content && !loading | |
| }" |
🤖 Prompt for AI Agents
In packages/core/src/components/Bubble/index.vue around lines 179-180, the
'no-content' class is being applied when content is empty even during loading,
which hides the loading indicator; change the class binding so 'no-content' is
applied only when there is no content and not loading (i.e., add the loading
check to the conditional) so the loading indicator remains visible.
Summary by CodeRabbit