fix: resolve warnings #957
Conversation
WalkthroughThis pull request encompasses modifications across multiple Vue components in the CodeRabbit project. The changes primarily involve refinements to component configurations, prop handling, and state management. Key modifications include adding localization support in Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/common/component/toolbar-built-in/ToolbarBaseIcon.vue (1)
Line range hint
21-23: Use a function returning an empty object for the default prop.Currently,
default: () => {}creates an empty function instead of an empty object, which can cause runtime errors if you attempt to access properties ofoptions. Update it to return an empty object for safety and clarity.- default: () => {} + default: () => ({})
🧹 Nitpick comments (11)
packages/configurator/src/tabs-group-configurator/TabsGroupConfigurator.vue (2)
81-82: Consolidate numeric handling or provide a fallback for non-numeric strings.
Currently,parseIntis used withprops.labelWidth, which can be either aNumberor aString. If a non-numeric string is provided,parseIntmight yieldNaN, resulting in a computed style of "NaNpx" and potential layout issues. Consider adding a fallback or validation for numeric inputs.
81-82: Revisit the default value.
The default value is now'63', which is a string. If the intention is purely numeric, a numeric default would be more direct, improving clarity and reducing parse concerns.packages/plugins/materials/src/meta/layout/src/Main.vue (1)
5-5: Consider providing a fallback for theheadercomponent.If
registryData?.components?.headeris undefined, this component reference will produce an empty or non-functional element. You may want to handle that state explicitly via a default component or conditional rendering.packages/layout/src/DesignSettings.vue (2)
25-26: RegisteringTinyTooltip
Newly registered tooltip component broadens the UI/UX capabilities. Ensure you have tests or QA to verify correct behavior for all tooltips.
41-42: ProvidingisCollapsedstate
Great use of dependency injection. Make sure descendants handleisCollapsedupdates consistently. Consider adding watchers or computed properties in child components if needed.packages/settings/styles/src/Main.vue (2)
120-121: Injecting and initializing reactive states
Usinginject('isCollapsed')andref(styleCategoryGroup)transitions the logic to a more dynamic approach. Verify thatactiveNamesremains synchronized withisCollapsedupdates elsewhere.
219-220: ExposeisCollapsedin setup return
Exposing the injected property from setup clarifies the component’s API. Ensure that consumers of this component do not unintentionally mutate it.packages/plugins/materials/src/meta/block/src/BlockGroup.vue (1)
171-171: Consider aligning event name with v-model conventions.Because you’re binding
v-modeltoselectedGroup, you might consider using the patternupdate:selectedGroupto keep everything consistent with Vue’s model emission conventions. However, this is optional and depends on your desired API design.packages/common/component/toolbar-built-in/ToolbarBaseButton.vue (3)
Line range hint
17-29: Consider enhancing prop definitions for better type safetyWhile the current prop definitions are functional, they could benefit from additional validation and more specific typing.
Consider applying these improvements:
props: { icon: { type: String, - default: '' + default: '', + validator: (value) => !value || typeof value === 'string' }, content: { type: String, - default: '' + default: '', + validator: (value) => !value || typeof value === 'string' }, options: { type: Object, default: () => ({}), + validator: (value) => { + if (!value) return true + return typeof value === 'object' && + (!('showDots' in value) || typeof value.showDots === 'boolean') + } } }
Line range hint
33-67: Consider refactoring styles for better maintainabilityThe current styles use
!importantflags and some hard-coded values which could be improved.Consider these improvements:
.toolbar-button { - background-color: var(--ti-lowcode-toolbar-button-bg) !important; - border: none !important; - min-width: 70px; - height: 26px; - line-height: 24px; - padding: 0 8px; - border-radius: 4px; - margin-right: 4px; + background-color: var(--ti-lowcode-toolbar-button-bg); + border: none; + min-width: var(--ti-lowcode-toolbar-button-min-width, 70px); + height: var(--ti-lowcode-toolbar-button-height, 26px); + line-height: var(--ti-lowcode-toolbar-button-line-height, 24px); + padding: var(--ti-lowcode-toolbar-button-padding, 0 8px); + border-radius: var(--ti-lowcode-toolbar-button-radius, 4px); + margin-right: var(--ti-lowcode-toolbar-button-spacing, 4px); }This change:
- Removes
!importantflags in favor of proper CSS specificity- Introduces CSS variables for all dimensions, making the component more customizable
- Maintains backward compatibility with default values
Line range hint
1-67: Add unit tests for the componentSince this is a fundamental UI component used in toolbars, it should have comprehensive test coverage to ensure reliability.
Would you like me to help generate unit tests that cover:
- Prop validation
- Slot rendering
- Icon display
- Optional dots display
- Content rendering
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
packages/common/component/MetaCodeEditor.vue(1 hunks)packages/common/component/MetaListItems.vue(1 hunks)packages/common/component/PluginBlockList.vue(1 hunks)packages/common/component/toolbar-built-in/ToolbarBaseButton.vue(1 hunks)packages/common/component/toolbar-built-in/ToolbarBaseIcon.vue(1 hunks)packages/configurator/src/tabs-group-configurator/TabsGroupConfigurator.vue(1 hunks)packages/layout/src/DesignSettings.vue(3 hunks)packages/plugins/i18n/src/Main.vue(1 hunks)packages/plugins/materials/src/meta/block/src/BlockGroup.vue(1 hunks)packages/plugins/materials/src/meta/block/src/BlockList.vue(0 hunks)packages/plugins/materials/src/meta/layout/src/Main.vue(2 hunks)packages/settings/styles/src/Main.vue(5 hunks)packages/settings/styles/src/components/border/BorderGroup.vue(4 hunks)
💤 Files with no reviewable changes (1)
- packages/plugins/materials/src/meta/block/src/BlockList.vue
✅ Files skipped from review due to trivial changes (2)
- packages/settings/styles/src/components/border/BorderGroup.vue
- packages/common/component/MetaListItems.vue
🔇 Additional comments (13)
packages/common/component/toolbar-built-in/ToolbarBaseIcon.vue (1)
11-11: Good improvement in passing down attributes.
Switching from extendAttrs to $attrs effectively passes all attributes from the parent, preserving flexibility. This approach helps keep the component open for extension and usage in more complex scenarios.
packages/common/component/PluginBlockList.vue (1)
208-208: Great fix to avoid undefined values for blockUsers.
Using an empty array default ensures blockUsers won't be undefined. Please consider potential immutability concerns: if any part of the code modifies the blockUsers array, you may want to wrap it in a reactive or ref to ensure its changes are tracked. Otherwise, this is a beneficial change as it avoids runtime errors.
packages/common/component/MetaCodeEditor.vue (1)
258-259: Add a watcher or reactive check for locale changes
You are returning locale in the setup function, which might be reactive because i18n.global.locale is typically a ref. Make sure that any UI elements using locale are updated seamlessly if the locale changes. If needed, add a watch on this reactive locale property to trigger re-render or updates accordingly.
packages/plugins/i18n/src/Main.vue (1)
117-117: Improved conditional rendering logic.
This change correctly ensures that the search-empty component is only displayed when isLoading is false, simplifying the logic and making it more explicit.
packages/plugins/materials/src/meta/layout/src/Main.vue (1)
59-59: Approved approach for default tab selection.
Using optional chaining with .options?.defaultTabId ensures a safe fallback to the first element in displayComponentIds. This maintains a clean and predictable user experience.
packages/layout/src/DesignSettings.vue (2)
5-5: Simplify logic with dynamic component usage
By removing the is-collapsed prop, this line now cleanly mounts the specified component without extra props. Ensure the removed prop isn't needed for internal logic.
18-19: Added provide and Tooltip imports
You’re now importing provide from Vue and Tooltip from @opentiny/vue. This appears consistent with the setup that shares state via dependency injection and uses a tooltip.
packages/settings/styles/src/Main.vue (4)
68-68: New usage of inject
Injecting a parent-provided property (isCollapsed) streamlines state sharing across the component tree. Verify that older references to isCollapsed prop are fully removed.
108-109: Removed isCollapsed prop
The props object is now empty, reflecting the shift to injecting isCollapsed. This reduces prop redundancy.
129-129: Conditional collapse logic
Limiting activeNames to a single item when collapsed is useful, but ensure possible corner cases (e.g., user reordering or disabling sections) are handled gracefully.
199-209: Watcher for isCollapsed
Automatically toggling activeNames based on isCollapsed state is intuitive. Ensure other logic that depends on these collapsible sections is updated accordingly.
packages/plugins/materials/src/meta/block/src/BlockGroup.vue (1)
171-171: Great fix to define the emitted event!
By declaring 'changeGroup' in the emits array, you eliminate Vue warnings and ensure that the emitted event is properly recognized. This is a standard best practice in Vue 3 for explicit event definitions.
packages/common/component/toolbar-built-in/ToolbarBaseButton.vue (1)
2-2: Verify attribute inheritance after removing extendAttrs
The removal of v-bind="extendAttrs" changes how attributes are inherited. Ensure that all necessary attributes from parent components are still properly passed down to the tiny-button.
Let's verify the usage of this component and any attributes being passed to it:
✅ Verification successful
Removal of extendAttrs binding is safe
Based on the verification:
- The previous implementation used
inject('extend-attributes')but there are no correspondingprovidecalls in the codebase for 'extend-attributes' - The parent component
ToolbarBase.vueusesToolbarBaseButtonthrough dynamic component rendering and only passes the reactivestateobject containingicon,content, andoptionsprops, which are all explicitly defined in both components - No additional attributes are being passed that would require the
extendAttrsbinding
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usage of ToolbarBaseButton component
rg -l "ToolbarBaseButton" | xargs rg -A 5 "<ToolbarBaseButton"
# Search for any custom attributes being passed
ast-grep --pattern '<ToolbarBaseButton $attr="$_">'
Length of output: 116
Script:
#!/bin/bash
# Let's try a different approach to find usages of ToolbarBaseButton
# First, find all Vue files that might import or use ToolbarBaseButton
rg -l "ToolbarBaseButton" --type vue
# Then search for the actual component usage with more context
rg -B 2 -A 5 "<ToolbarBaseButton" --type vue
# Also search for imports of ToolbarBaseButton
rg "import.*ToolbarBaseButton" --type vue
# Check the component's previous implementation
git show HEAD~1:packages/common/component/toolbar-built-in/ToolbarBaseButton.vue
Length of output: 1639
Script:
#!/bin/bash
# Let's try searching without file type restriction
# Search for files containing ToolbarBaseButton
rg -l "ToolbarBaseButton"
# Search for component usage with context
rg -B 2 -A 5 "<ToolbarBaseButton"
# Search for imports
rg "import.*ToolbarBaseButton"
# Search for any provide/inject usage of 'extend-attributes'
rg "extend-attributes"
Length of output: 274
Script:
#!/bin/bash
# Let's check the ToolbarBase.vue file to see how ToolbarBaseButton is used
cat packages/common/component/ToolbarBase.vue
# Also search for any provide usage in parent components
rg -B 5 -A 5 "provide\(['\"]extend-attributes"
Length of output: 1719
packages/settings/styles/src/components/inputs/NumericSelect.vue
Outdated
Show resolved
Hide resolved
* fix: resolve warnings * fix: add access guard avoid runtime errors * fix: change placeholder prop type * fix: resolve type check * fix: toolbarbaseicon props default value
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
localeproperty to enhance localization in the Meta Code Editor.changeGroupin the Block Group component for improved interactivity.TinyTooltipcomponent to enhance tooltip functionality in Design Settings.Improvements
isCollapsedfrom the parent context.labelWidthproperty in Tabs Group Configurator for greater flexibility.Bug Fixes
Refactor
extendAttrsvariable in ToolbarBaseButton and ToolbarBaseIcon for cleaner code.