fix：adjust some styles #884
Conversation
WalkthroughThis pull request introduces modifications across several Vue components, primarily focusing on enhancing styling, structure, and functionality. Key changes include the addition of an Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (5)
packages/plugins/state/src/StateFullscreenHead.vue (1)
40-44: Consider adding line-height for better vertical alignment.
The styling uses design system variables appropriately, but could benefit from explicit line-height for better vertical alignment with the close icon.
Consider adding line-height:
.title {
font-size: var(--te-base-font-size-base);
color: var(--te-common-text-primary);
font-weight: var(--te-base-font-weight-7);
+ line-height: 30px; /* Match parent's height for vertical centering */
}Also, could you verify if font-weight-7 is the intended weight for this title? It seems unusually heavy.
packages/plugins/state/src/CreateStore.vue (3)
Line range hint 13-29: Improve Monaco editor event handling and validation.
Several issues with the current implementation:
- Both
@editorDidMountand@changeevents call the same method, which is redundant. - The method name
editorDidMountis misleading as it performs content validation. - The validation doesn't handle JSON parse errors.
Consider this improved implementation:
<monaco-editor
ref="variableEditor"
class="store-editor"
:value="editorCode"
:showFormatBtn="true"
:options="{
language: 'json',
overviewRulerBorder: false,
renderLineHighlightOnlyWhenFocus: true
}"
- @editorDidMount="editorDidMount"
- @change="editorDidMount"
+ @editorDidMount="handleEditorMount"
+ @change="validateStateContent"
>And update the methods:
const validateStateContent = () => {
try {
const content = variableEditor.value.getEditor().getValue();
const parsed = JSON.parse(content);
return typeof parsed === 'object' && parsed !== null;
} catch (e) {
return false;
}
};
const handleEditorMount = (editor) => {
// Handle editor initialization if needed
};Line range hint 142-157: Enhance store name validation.
The current validation could be improved to handle more edge cases and provide clearer error messages.
Consider enhancing the validation:
const validateName = (rule, name, callback) => {
let errorMessage = ''
let isSameState = Object.keys(props.dataSource).includes(name)
+ const reservedKeywords = ['break', 'case', 'catch', 'class', 'const', 'continue',
+ 'debugger', 'default', 'delete', 'do', 'else', 'export', 'extends', 'finally',
+ 'for', 'function', 'if', 'import', 'in', 'instanceof', 'new', 'return', 'super',
+ 'switch', 'this', 'throw', 'try', 'typeof', 'var', 'void', 'while', 'with', 'yield']
if (!name) {
errorMessage = 'store 属性名称未定义'
+ } else if (reservedKeywords.includes(name)) {
+ errorMessage = 'store 属性名称不能使用 JavaScript 保留关键字'
} else if (!verifyJsVarName(name)) {
- errorMessage = ' store 属性名称只能以字母或下划线开头且仅包含数字字母及下划线'
+ errorMessage = name.match(/^\d/) ?
+ 'store 属性名称不能以数字开头' :
+ 'store 属性名称只能包含字母、数字和下划线，且必须以字母或下划线开头'
} else if (isSameState && (props.flag !== 'update' || name !== props.updateKey)) {
errorMessage = '已存在同名 store 属性'
}
errorMessage ? callback(new Error(errorMessage)) : callback()
emit('nameInput', errorMessage)
}Line range hint 187-206: Optimize state management and AST operations.
The current implementation could benefit from improved error handling and performance optimizations.
Consider these improvements:
+ import { onBeforeUnmount } from 'vue'
const saveMethods = (editor) => {
+ try {
const storeEditor = editor === 'gettersEditor' ? gettersEditor : actionsEditor
let gettersMap = {}
const editorContent = storeEditor?.value?.getEditor()?.getValue()
const ast = string2Ast(editorContent)
ast.program.body.forEach((declaration) => {
const name = declaration?.id?.name
const content = ast2String(declaration).trim()
Object.assign(gettersMap, saveMethod({ name, content }))
})
return gettersMap
+ } catch (error) {
+ console.error('Failed to parse methods:', error)
+ return {}
+ }
}
+ // Cleanup editor instances
+ onBeforeUnmount(() => {
+ variableEditor.value?.getEditor()?.dispose()
+ gettersEditor.value?.getEditor()?.dispose()
+ actionsEditor.value?.getEditor()?.dispose()
+ })packages/plugins/help/src/HelpIcon.vue (1)
Line range hint 109-124: Consider cleanup of timer in component unmount.
The toolTipTimer should be cleared when the component is unmounted to prevent memory leaks.
Add the following cleanup:
+import { reactive, onMounted, ref, onBeforeUnmount } from 'vue'
export default {
// ... existing code ...
setup() {
// ... existing code ...
+ onBeforeUnmount(() => {
+ clearTimeout(toolTipTimer)
+ })
// ... existing code ...
}
}📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
packages/design-core/assets/full-screen.svgis excluded by!**/*.svgpackages/design-core/assets/全屏.svgis excluded by!**/*.svg
📒 Files selected for processing (7)
- packages/common/component/MonacoEditor.vue (2 hunks)
- packages/layout/src/ToolbarCollapse.vue (1 hunks)
- packages/plugins/help/src/HelpIcon.vue (4 hunks)
- packages/plugins/state/src/CreateStore.vue (1 hunks)
- packages/plugins/state/src/CreateVariable.vue (2 hunks)
- packages/plugins/state/src/StateFullscreenHead.vue (2 hunks)
- packages/toolbars/save/src/Main.vue (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/plugins/state/src/CreateVariable.vue
🔇 Additional comments (6)
packages/plugins/state/src/StateFullscreenHead.vue (1)
3-3: LGTM! Semantic class name addition.
The addition of the "title" class name is semantic and follows good CSS naming practices.
packages/layout/src/ToolbarCollapse.vue (1)
2-2: LGTM! Verify consistent popover styling across the application.
The addition of :visible-arrow="false" to hide the popover arrow is valid and aligns with the style adjustments objective.
Let's verify if this style is consistently applied across other popover instances:
✅ Verification successful
:visible-arrow="false" is consistently used across similar hover popovers
The search results show that hiding the popover arrow is a common pattern in the codebase, particularly for hover-triggered popovers in toolbar and menu components. Examples:
packages/toolbars/save/src/Main.vue::visible-arrow="false"with hover triggerpackages/plugins/help/src/HelpIcon.vue::visible-arrow="false"for menu popoverpackages/plugins/state/src/EditorI18nTool.vue::visible-arrow="false"for tool popover
The change aligns with the established pattern in the codebase for similar UI components.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if other popovers in the codebase have consistent arrow visibility settings
# Expected: Find other popover instances to ensure consistent styling
# Search for tiny-popover usage in vue files
rg -t vue 'tiny-popover' -A 2 -B 2
# Search specifically for visible-arrow prop usage
rg -t vue 'visible-arrow' -A 2 -B 2
Length of output: 126
Script:
#!/bin/bash
# Let's try without file type filtering since rg doesn't recognize .vue
# Search for tiny-popover usage
rg 'tiny-popover' -A 2 -B 2
# Search specifically for visible-arrow prop usage
rg 'visible-arrow' -A 2 -B 2
Length of output: 52395
packages/toolbars/save/src/Main.vue (1)
203-212: LGTM! Clean and consistent styling structure.
The style changes improve the visual hierarchy and spacing in the save settings popover. The padding, font sizes, and margins create a well-balanced layout.
packages/plugins/state/src/CreateStore.vue (1)
257-259: LGTM: Style adjustment improves spacing.
The removal of padding from .tiny-collapse-item__content prevents double padding since the parent wrapper already has padding, resulting in cleaner spacing.
packages/plugins/help/src/HelpIcon.vue (2)
64-64: LGTM: Component registration is correct.
The Popover component is properly imported and registered.
Also applies to: 75-75
349-349: LGTM: Style changes maintain consistency.
The styling updates properly utilize CSS variables for theming and maintain consistent spacing through the help box items.
Also applies to: 355-371
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
packages/toolbars/lang/src/Main.vue (2)
3-3: Consider internationalizing the tooltip text.The hardcoded Chinese tooltip "画布中英文切换" should be internationalized since this is a language switcher component.
- content="画布中英文切换" + :content="t('canvas.languageSwitcher')"
Line range hint
39-49: Consider making language options configurable.The language options are currently hardcoded in the component. Consider making them configurable through props or configuration to:
- Support additional languages in the future
- Allow customization of labels
- Maintain language configurations in a central location
+ langOptions: { + type: Array, + default: () => [ + { value: 'zh_CN', label: '中文' }, + { value: 'en_US', label: 'English' } + ] + }, options: { type: Object, default: () => ({}) } }, setup(props) { const langVal = ref('zh_CN') const { post, data } = useBroadcastChannel({ name: props.langChannel }) watch(data, () => { langVal.value = data.value }) - const langOptions = [ - { - value: 'zh_CN', - label: '中文' - }, - { - value: 'en_US', - label: 'English' - } - ] const changeLang = () => { - langVal.value = langVal.value === langOptions[0].value ? langOptions[1].value : langOptions[0].value + const currentIndex = props.langOptions.findIndex(opt => opt.value === langVal.value) + const nextIndex = (currentIndex + 1) % props.langOptions.length + langVal.value = props.langOptions[nextIndex].value post(langVal.value) }packages/common/component/ToolbarBase.vue (2)
Line range hint
39-42: Fix the options prop default value.The current default value
() => {}returnsundefinedinstead of an empty object. This could cause runtime errors when accessing properties likeoptions.renderTypeoroptions.collapsed.Apply this fix:
options: { type: Object, - default: () => {} + default: () => ({}) }
Default options prop needs correction to prevent runtime errors
The
optionsprop default value is incorrectly set to() => {}which returnsundefined. This causes potential runtime errors ingetRender()when accessingprops.options.renderType. The default value should be an empty object() => ({}).
packages/common/component/ToolbarBase.vue:33: Change default value of options prop from() => {}to() => ({})🔗 Analysis chain
Line range hint
8-8: Verify error handling in getRender function.The
getRenderfunction assumesprops.optionsis always an object when accessingprops.options.renderType. With the current default value issue, this could lead to runtime errors.Let's verify if there are any error cases in the codebase:
Also applies to: 41-56
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for ToolbarBase usage without options prop # or with invalid renderType values # Search for ToolbarBase component usage rg -A 3 '<(?:ToolbarBase|toolbar-base)' # Search for any error handling related to ToolbarBase rendering rg -A 3 'renderType.*(?:error|catch|try|handle)'Length of output: 4255
Script:
#!/bin/bash # Let's check the implementation of getRender function and its error handling ast-grep --pattern 'getRender() { $$$ }' # Also check for any default props or options initialization ast-grep --pattern 'props: { $$$ }' # And check for any renderType related code rg -A 3 'renderType'Length of output: 2904
Script:
#!/bin/bash # Let's check the complete ToolbarBase.vue file to understand the props definition and default values cat packages/common/component/ToolbarBase.vue # Also check if there are any default options in the meta files rg -l 'export default' packages/toolbars/*/meta.js | xargs catLength of output: 5150
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (3)
packages/design-core/assets/cn-en.svgis excluded by!**/*.svgpackages/design-core/assets/refresh.svgis excluded by!**/*.svgpackages/design-core/assets/team-collaboration.svgis excluded by!**/*.svg
📒 Files selected for processing (8)
packages/common/component/ToolbarBase.vue(1 hunks)packages/layout/src/ToolbarCollapse.vue(2 hunks)packages/theme/base/src/base.less(1 hunks)packages/theme/base/src/component-common.less(1 hunks)packages/toolbars/collaboration/meta.js(1 hunks)packages/toolbars/lang/meta.js(1 hunks)packages/toolbars/lang/src/Main.vue(1 hunks)packages/toolbars/save/src/Main.vue(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/theme/base/src/base.less
- packages/toolbars/collaboration/meta.js
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/layout/src/ToolbarCollapse.vue
- packages/toolbars/save/src/Main.vue
🔇 Additional comments (4)
packages/toolbars/lang/meta.js (1)
7-7: LGTM! Verify the new icon asset exists.
The consolidation of language icons into a single combined icon is a good UI optimization.
Let's verify the existence of the new icon asset:
✅ Verification successful
Icon asset verified: cn-en.svg exists in design-core assets
The new combined language icon asset cn-en.svg is present at packages/design-core/assets/cn-en.svg, confirming that the icon configuration change is properly supported by the required asset.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the new combined language icon asset exists
# Expected: Find references to the 'cn-en' icon in asset files or style definitions
# Search for the icon in asset files
fd -e svg -e png -e jpg -e jpeg . | rg -i "cn-en"
# Search for the icon name in style files
rg -t css -t scss -t less "cn-en"
Length of output: 150
packages/toolbars/lang/src/Main.vue (1)
4-4: LGTM: Robust icon fallback implementation.
The fallback mechanism options.icon.default || options.icon ensures graceful handling of icon configuration.
packages/common/component/ToolbarBase.vue (1)
74-77: LGTM! Clean and semantic CSS implementation.
The new flexbox layout provides proper vertical alignment for toolbar items.
packages/theme/base/src/component-common.less (1)
463-465: LGTM! Clean popover styling update.
The removal of the border in favor of a box-shadow creates a more modern, floating appearance for the popover component. The use of CSS variables maintains consistency with the design system.
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Release Notes
New Features
tiny-popovercomponent for help interactions, replacing the previous help box.Improvements
Bug Fixes
Chores