feat: enhance the divider dialogbox treemenu function #2758
Conversation
WalkthroughThis pull request introduces comprehensive updates to multiple components, with a primary focus on the Divider and Dialog Box components. The changes span across various files, including demos, renderless logic, type definitions, and styling. The modifications enhance component flexibility, add new properties, and improve styling options for both desktop and mobile-first implementations. Changes
Sequence DiagramsequenceDiagram
participant User
participant Component
participant RenderlessLogic
participant Styles
User->>Component: Set custom properties
Component->>RenderlessLogic: Pass props
RenderlessLogic->>RenderlessLogic: Compute styles
RenderlessLogic->>Styles: Apply computed styles
Styles-->>Component: Render styled component
Possibly related PRs
Suggested labels
Poem
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 (
|
WalkthroughThis pull request enhances the functionality of the divider, dialog box, and tree menu components in the Tiny Vue library. It introduces new features such as custom styles, no-animation options, and additional configuration for dividers. The changes also include updates to demos, documentation, and internal utilities to support these enhancements. Changes
|
![petercat-assistant[bot]](https://avatars.githubusercontent.com/in/875736?s=60&v=4){kind=small}
| @@ -58,7 +58,7 @@ const pickerOptions = { | |||
| } | |||
| const yearPickerOptions = { | |||
| disabledDate(year) { | |||
| return year > 2025 || year < 2022 | |||
| return year.getTime() > new Date(2025, 0, 1, 0).getTime() || year.getTime() < new Date(2022, 0, 1, 0).getTime() | |||
There was a problem hiding this comment.
The change from comparing years directly to using getTime() is necessary to ensure accurate date comparisons, especially when dealing with Date objects. This prevents potential logical errors when comparing Date objects directly.
| @@ -62,7 +62,9 @@ export default { | |||
| }, | |||
| yearPickerOptions: { | |||
| disabledDate(year) { | |||
| return year > 2025 || year < 2022 | |||
| return ( | |||
There was a problem hiding this comment.
Using getTime() for date comparisons ensures that the comparison is made on the exact time value, which is crucial for accurate logic when dealing with Date objects.
| @@ -116,7 +116,7 @@ export const renderless = ( | |||
|
|
|||
| onBeforeUnmount(unbindDialogEvent) | |||
|
|
|||
| watch(() => props.rules, api.watchRules) | |||
| watch(() => props.rules, api.watchRules, { deep: props.validateOnRuleChange === 'deep' }) | |||
There was a problem hiding this comment.
The addition of the deep option for validateOnRuleChange allows for more granular control over validation, ensuring that nested rule changes trigger validations as expected.
|
[e2e-test-warn] The title of the Pull request should look like "fix(vue-renderless): [action-menu, alert] fix xxx bug". Please make sure you've read our contributing guide |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
🧹 Nitpick comments (49)
packages/renderless/src/rich-text-editor/vue.ts (1)
264-264: Consider passing editor instance to destroy event.For consistency with other events and to provide context for cleanup operations, consider passing the editor instance to the destroy event.
- onDestroy() { - emit('destroy') + onDestroy({ editor }) { + emit('destroy', { editor })packages/vue-directive/src/highlight-query.ts (2)
Line range hint
13-38: Consider optimizing performance for multiple matchesThe current implementation processes one match at a time and uses recursion for multiple matches in the same text node. This approach could be inefficient for text with many matches.
Consider optimizing by processing all matches in a single pass:
function editNodes(el: HTMLElement, nodes: HTMLElement[], query: string) { nodes.forEach((node) => { const content = node.textContent as string const matches = [] as { index: number; length: number }[] let index = 0 // Find all matches in a single pass while ((index = content.indexOf(query, index)) !== -1) { matches.push({ index, length: query.length }) index += query.length } if (matches.length === 0) return // Create document fragment to minimize DOM operations const fragment = document.createDocumentFragment() let lastIndex = 0 matches.forEach(({ index, length }) => { // Add text before match if (index > lastIndex) { fragment.appendChild( document.createTextNode(content.substring(lastIndex, index)) ) } // Add highlighted match const span = document.createElement('span') span.classList.add('tiny-hl-query-node') span.textContent = content.substring(index, index + length) fragment.appendChild(span) lastIndex = index + length }) // Add remaining text if (lastIndex < content.length) { fragment.appendChild( document.createTextNode(content.substring(lastIndex)) ) } // Replace node content with fragment in a single operation node.textContent = '' node.appendChild(fragment) }) }This optimization:
- Eliminates recursion
- Reduces string operations
- Minimizes DOM manipulations using DocumentFragment
- Improves performance for text with multiple matches
Line range hint
1-12: Add input validation and error handlingThe code would benefit from improved error handling and input validation.
Consider adding these safeguards:
function walk(el: HTMLElement | null, filter: (node: HTMLElement) => boolean) { if (!el) { console.warn('highlight-query: Element is null') return [] } try { const walker = document.createTreeWalker(el, NodeFilter.SHOW_TEXT) const matchNodes = [] as HTMLElement[] while (walker.nextNode()) { const node = walker.currentNode as HTMLElement if (filter(node)) matchNodes.push(node) } return matchNodes } catch (error) { console.error('highlight-query: Error walking DOM', error) return [] } } const mounted = (el: HTMLElement, binding: { value: string | null | undefined }) => { if (!el) { console.warn('highlight-query: Element is null') return } try { el.classList.toggle('tiny-hl-query', true) const query = binding.value?.trim() || '' edit(el, query) } catch (error) { console.error('highlight-query: Error in mounted hook', error) } }This adds:
- Null checks for DOM elements
- Validation for query strings
- Error handling for DOM operations
- Meaningful error messages
Also applies to: 71-83
examples/sites/demos/pc/app/divider/status.vue (1)
1-9: Consider adding comments about the expected visual appearance.The template effectively demonstrates all status variations. Consider adding comments to describe the expected visual appearance (e.g., colors) for each status to help developers understand the intended outcome.
<template> <div> + <!-- Renders a default divider with neutral styling --> <tiny-divider> default </tiny-divider> + <!-- Renders a divider with green success styling --> <tiny-divider status="success"> success </tiny-divider> + <!-- Renders a divider with red error styling --> <tiny-divider status="error"> error </tiny-divider> + <!-- Renders a divider with orange warning styling --> <tiny-divider status="warning"> warning </tiny-divider> + <!-- Renders a divider with blue info styling --> <tiny-divider status="info"> info </tiny-divider> </div> </template>packages/renderless/types/tree-menu.type.ts (1)
57-57: Consider grouping related methods in the interface.The new methods
getTree,handleToggleMenu, andcomputedTreeStyleare spread across the interface. Consider grouping them together based on their functionality (e.g., tree manipulation methods, styling methods).export interface ITreeMenuApi { // ... other methods ... // Tree manipulation methods getTree: ReturnType<typeof getTree> handleToggleMenu: ReturnType<typeof handleToggleMenu> // Styling methods computedTreeStyle: ReturnType<typeof computedTreeStyle> }Also applies to: 67-68
packages/renderless/src/tree-menu/index.ts (1)
30-35: Consider adding TypeScript type annotations.The function parameters and return type should be explicitly typed for better type safety and documentation.
export const getTree = - ({ vm }) => + ({ vm }: { vm: Vue }) => - () => { + (): Tree | null => { if (!vm.$refs.tree) { console.warn('Tree ref is not available') return null } return vm.$refs.tree }packages/renderless/src/divider/vue.ts (1)
17-18: Remove unnecessary type annotationsThe types for
defaultMarginandverticalMargincan be inferred from the assigned string literals, making the explicit type annotations redundant.Apply this diff to remove the unnecessary type annotations:
- const defaultMargin: string = '16px 0' + const defaultMargin = '16px 0' - const verticalMargin: string = '0 8px' + const verticalMargin = '0 8px'🧰 Tools
🪛 eslint
[error] 17-17: Type string trivially inferred from a string literal, remove type annotation.
(@typescript-eslint/no-inferrable-types)
[error] 18-18: Type string trivially inferred from a string literal, remove type annotation.
(@typescript-eslint/no-inferrable-types)
packages/mobile/vue-common/src/adapter/vue3/index.ts (7)
32-32: Simplify code using optional chainingYou can simplify the expression
(view && view.value)by using optional chainingview?.value.Apply this diff to use optional chaining:
- return () => hooks.h((view && view.value) || component, { ref: 'modeTemplate', ...props, ...attrs, ...extend }, slots) + return () => hooks.h((view?.value) || component, { ref: 'modeTemplate', ...props, ...attrs, ...extend }, slots)🧰 Tools
🪛 Biome (1.9.4)
[error] 32-32: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
66-66: Simplify code using optional chainingYou can simplify the expression
router && router.currentRoute.valueby using optional chainingrouter?.currentRoute.value.Apply this diff to use optional chaining:
- const route = router && router.currentRoute.value + const route = router?.currentRoute.value
79-79: Simplify code using optional chainingYou can simplify the expression
(vm.subTree && vm.subTree.children)by using optional chainingvm.subTree?.children.Apply this diff to use optional chaining:
- const children = (vm.subTree && vm.subTree.children) || vm.children + const children = (vm.subTree?.children) || vm.children🧰 Tools
🪛 Biome (1.9.4)
[error] 79-82: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
84-86: Simplify code using optional chainingIn the
broadcastfunction, you can simplify property accesses using optional chaining.Apply this diff to use optional chaining:
- const name = child.type && child.type.componentName - const component = child.component + const name = child.type?.componentName + const component = child.component🧰 Tools
🪛 Biome (1.9.4)
[error] 84-86: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
100-100: Simplify code using optional chainingYou can simplify the expression
parent.type && parent.type.componentNameusing optional chainingparent.type?.componentName.Apply this diff to use optional chaining:
- let name = parent.type && parent.type.componentName + let name = parent.type?.componentName🧰 Tools
🪛 Biome (1.9.4)
[error] 100-100: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
120-120: Simplify code using optional chainingYou can simplify the expression
vm.parent && vm.parentusing optional chainingvm.parent?.parent.Apply this diff to use optional chaining:
- return vm.parent.type.name === 'AsyncComponentWrapper' && vm.parent.parent ? vm.parent.parent : vm.parent + return vm.parent.type.name === 'AsyncComponentWrapper' && vm.parent?.parent ? vm.parent.parent : vm.parent🧰 Tools
🪛 Biome (1.9.4)
[error] 120-120: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
268-269: Avoid assignments within expressionsAssigning within expressions can be confusing. It's clearer to perform the assignment in a separate statement.
Apply this diff to separate the assignment:
- vm.$once = $emitter.once = ... + $emitter.once = ... + vm.$once = $emitter.once🧰 Tools
🪛 Biome (1.9.4)
[error] 268-269: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
examples/sites/demos/mobile-first/app/divider/basic-usage.vue (1)
3-7: Consider internationalizing the demo textThe Chinese text "基础使用" and "分割线" should be internationalized to support multiple languages.
Consider using i18n translations:
- <p>基础使用</p> - <tiny-divider> </tiny-divider> - <br /> - <p>带文本分割线</p> - <tiny-divider> 分割线 </tiny-divider> + <p>{{ t('basic.usage') }}</p> + <tiny-divider> </tiny-divider> + <br /> + <p>{{ t('divider.withText') }}</p> + <tiny-divider> {{ t('divider.text') }} </tiny-divider>examples/sites/demos/pc/app/divider/divider-type.vue (1)
3-5: Improve text consistency and formattingThe text formatting is inconsistent with mixed English/Chinese and inconsistent suffixes.
Consider standardizing the text format:
- <tiny-divider>solid线</tiny-divider> - <tiny-divider border-style="dashed">dashed线</tiny-divider> - <tiny-divider border-style="dotted">dotted线</tiny-divider> + <tiny-divider>Solid</tiny-divider> + <tiny-divider border-style="dashed">Dashed</tiny-divider> + <tiny-divider border-style="dotted">Dotted</tiny-divider>examples/sites/demos/mobile-first/app/divider/content-position.vue (1)
3-5: Add offset values to demonstrate full positioning capabilitiesThe demo could better showcase the positioning features by including offset values.
Consider adding offset demonstrations:
- <tiny-divider> 分割线 </tiny-divider> - <tiny-divider content-position="left"> 分割线 </tiny-divider> - <tiny-divider content-position="right"> 分割线 </tiny-divider> + <tiny-divider> {{ t('divider.text') }} </tiny-divider> + <tiny-divider content-position="left" :offset="20"> {{ t('divider.text') }} </tiny-divider> + <tiny-divider content-position="right" :offset="80"> {{ t('divider.text') }} </tiny-divider>examples/sites/demos/mobile-first/app/divider/type.vue (1)
1-17: Consider consolidating demo implementationsThe type.vue and divider-type.vue files demonstrate the same functionality but are implemented separately for mobile-first and PC contexts.
Consider:
- Creating a shared base component that both demos can extend
- Using responsive design to handle both mobile and desktop layouts
- Implementing a single source of truth for border style demonstrations
examples/sites/demos/pc/app/divider/status-composition-api.vue (1)
3-7: Consider internationalizing the status text contentThe status text appears to be hardcoded in English. Consider using i18n for better internationalization support.
examples/sites/demos/mobile-first/app/divider/divider-margin.vue (2)
3-7: Consider using CSS variables for margin valuesInstead of hardcoding the margin value "20px 0", consider using CSS variables for better maintainability and consistency across the application.
11-19: Consider migrating to Composition APIThe component uses the Options API. Consider migrating to the Composition API with
<script setup>for consistency with other components likestatus-composition-api.vue.Example migration:
-<script> +<script setup> import { Divider } from '@opentiny/vue' - -export default { - components: { - TinyDivider: Divider - } -} </script>examples/sites/demos/pc/app/divider/content-position.vue (1)
3-3: Consider internationalizing the text contentThe Chinese text strings should be internationalized to support multiple languages.
Also applies to: 5-5, 7-7
internals/cli/src/shared/config.ts (1)
3-3: Fix typo in constant nameThe constant name 'EXTENERAL' appears to be misspelled. Should be 'EXTERNAL'.
examples/sites/demos/pc/app/divider/custom-style.vue (2)
3-3: Consider internationalizing the text contentThe Chinese text strings should be internationalized to support multiple languages.
Also applies to: 5-5, 9-9
4-4: Well-structured demonstration of styling optionsExcellent examples showcasing various customization options:
- Line colors
- Text colors and background
- Font sizes
- Height and margin adjustments
Consider using CSS variables for the color values to maintain theme consistency.
Also applies to: 6-8, 10-10
examples/sites/demos/mobile-first/app/divider/vertical.vue (1)
4-22: Enhance accessibility and internationalization
- Consider internationalizing the Chinese text strings
- Remove unnecessary spaces in divider content:
- <tiny-divider direction="vertical" status="success"> </tiny-divider> + <tiny-divider direction="vertical" status="success"></tiny-divider>packages/vue/src/divider/src/pc.vue (1)
34-47: Consider adding prop validations and documentation.While the new props enhance component configurability, consider:
- Adding validation for numeric props (margin, offset, fontSize, height)
- Including JSDoc comments to document the purpose and expected values of each prop
Example:
props: [ ...props, 'direction', 'color', 'borderStyle', 'contentPosition', 'contentColor', 'contentBackgroundColor', + /** @prop {string} status - Component status (default, success, warning, error, info) */ 'status', + /** @prop {string|number} margin - Component margin (e.g., '10px', 10) */ 'margin', + /** @prop {string|number} offset - Content offset (e.g., '5px', 5) */ 'offset', + /** @prop {string|number} fontSize - Text font size (e.g., '14px', 14) */ 'fontSize', + /** @prop {string|number} height - Component height (e.g., '1px', 1) */ 'height' ],packages/vue/src/divider/src/mobile-first.vue (1)
25-47: Consider type safety improvements.The TypeScript implementation could be enhanced:
- The
anyreturn type in setup could be more specific- Props could use more specific types instead of string literals
- setup(props, context): any { + setup(props, context) { + return setup({ props, context, renderless, api, classes }) as { + state: { rootStyle: object; lineStyle: object; textStyle: object } + } }packages/renderless/types/divider.type.ts (1)
32-41: Consider enhancing type safety for style properties.The IDividerStyle interface could be improved:
- Use more specific types for numeric values
- Add unit validation for CSS properties
export interface IDividerStyle { - margin?: string | number + margin?: string | `${number}px` | `${number}%` - height?: string | number + height?: string | `${number}px` | `${number}%` - fontSize?: string | number + fontSize?: string | `${number}px` | `${number}rem` right?: string | number left?: string | number borderLeftStyle?: string | number borderTopStyle?: string | number borderTopColor?: string | number }packages/vue/src/divider/src/index.ts (1)
11-53: Consider enhancing prop validations.The dividerProps definition is comprehensive but could be improved:
- Add validators for numeric string props
- Consider using an enum for status values
export const dividerProps = { // ... existing props fontSize: { type: String, default: () => '14px', + validator: (value: string) => /^\d+(\.\d+)?(px|rem|em)$/.test(value) }, margin: { type: String, + validator: (value: string) => /^\d+(\.\d+)?(px|%|rem|em)$/.test(value) }, offset: { type: String, + validator: (value: string) => /^\d+(\.\d+)?(px|%|rem|em)$/.test(value) }, height: { type: String, + validator: (value: string) => /^\d+(\.\d+)?(px|%|vh)$/.test(value) }, status: { type: String, values: ['default', 'success', 'warning', 'error', 'info'], default: () => 'default' } }packages/renderless/src/divider/index.ts (1)
12-24: Consider adding status-based color handling.The
computedLineStylefunction handles border style and color well, but could be enhanced to support status-based colors (e.g., success, warning, error states) as mentioned in the AI summary.export const computedLineStyle = ({ props }: Pick<IDividerRenderlessParams, 'props'>) => (): IDividerStyle => { const lineStyle = { borderTopStyle: props.borderStyle } + // Status-based colors take precedence over custom colors + if (props.status && statusColors[props.status]) { + Object.assign(lineStyle, { borderTopColor: statusColors[props.status] }) + } else if (props.color) { Object.assign(lineStyle, { borderTopColor: props.color }) } return lineStyle }examples/sites/demos/pc/app/date-picker/disabled.vue (1)
65-67: Optimize date comparison logic.While the timestamp comparison is more precise, creating new Date objects on every check is inefficient. Consider memoizing the boundary dates.
yearPickerOptions: { + // Memoize boundary dates + boundaryDates: { + min: new Date(2022, 0, 1, 0).getTime(), + max: new Date(2025, 0, 1, 0).getTime() + }, disabledDate(year) { return ( - year.getTime() > new Date(2025, 0, 1, 0).getTime() || year.getTime() < new Date(2022, 0, 1, 0).getTime() + year.getTime() > this.boundaryDates.max || year.getTime() < this.boundaryDates.min ) } }packages/renderless/types/dropdown.type.ts (1)
53-54: Add JSDoc comments for new methods.The new focus management methods would benefit from documentation explaining their purpose and usage.
+ /** Sets the focus state to true */ toggleFocusOnTrue: () => void + /** Sets the focus state to false */ toggleFocusOnFalse: () => voidpackages/vue/src/form/src/index.ts (1)
36-38: Document the 'deep' validation option.The new 'deep' option for validateOnRuleChange needs documentation to explain its purpose and behavior.
validateOnRuleChange: { type: [Boolean, String], + /** + * Controls validation behavior when rules change + * @param {boolean|'deep'} value - Set to 'deep' for recursive validation + * @since 3.21.0 + */ validator: (value: string | boolean) => typeof value === 'boolean' || ['deep'].includes(value), default: true },examples/sites/demos/pc/app/divider/webdoc/divider.js (1)
57-81: Well-structured new demos.The new demos (
divider-typeandstatus) are well-documented with clear descriptions of their functionality. However, consider adding examples of possible values for thestatusattribute in the description.- 'zh-CN': '通过<code>status</code>属性设置分割线显示状态颜色。', + 'zh-CN': '通过<code>status</code>属性设置分割线显示状态颜色。可选值：success、warning、error。', - 'en-US': 'Use the <code>status</code> attribute to set the color of the display status of the splitter line.' + 'en-US': 'Use the <code>status</code> attribute to set the color of the display status of the splitter line. Available values: success, warning, error.'packages/vue/src/dialog-box/src/index.ts (1)
126-133: Well-defined new properties.The new properties enhance dialog box customization:
dialogTransition: Allows custom transition namesnoAnimation: Enables/disables animationscustomStyle: Enables custom styling via objectConsider adding JSDoc comments to document these properties and their usage.
+ /** + * Custom transition name for dialog animations + * @default '' + */ dialogTransition: { type: String, default: '' }, + /** + * Disable all dialog animations when true + * @default false + */ noAnimation: { type: Boolean, default: false }, + /** + * Custom styles object to be applied to the dialog + * @default {} + */ customStyle: { type: Object, default: () => ({}) }packages/renderless/src/dropdown/vue.ts (1)
89-91: Well-structured focus management.The focus management is implemented with clear, single-responsibility methods. Consider adding TypeScript type annotations for better type safety.
- clickOutside: clickOutside({ props, api }), - toggleFocusOnTrue: toggleFocus({ state, value: true }), - toggleFocusOnFalse: toggleFocus({ state, value: false }) + clickOutside: clickOutside({ props, api }): void, + toggleFocusOnTrue: toggleFocus({ state, value: true }): void, + toggleFocusOnFalse: toggleFocus({ state, value: false }): voidexamples/sites/demos/pc/app/dialog-box/double-dialog-height-composition-api.vue (2)
1-38: Well-structured template with nested dialogs.The template structure is clear and follows Vue.js best practices. However, consider extracting hard-coded values to constants.
+<script setup> +const PARENT_DIALOG_WIDTH = '30%' +const CHILD_DIALOG_WIDTH = '300px' +</script> + <template> <tiny-dialog-box :visible="boxVisibility" right-slide @update:visible="boxVisibility = $event" title="父弹窗" - width="30%" + :width="PARENT_DIALOG_WIDTH" :custom-style="customStyle1" > <!-- ... --> <tiny-dialog-box <!-- ... --> - width="300px" + :width="CHILD_DIALOG_WIDTH" >
119-128: Improve animation timing management.The dialog close animation uses hardcoded timeouts. Consider:
- Using CSS transitions for animations
- Extracting timing values to constants
- Using Vue's
<Transition>component for smoother animations+const CLOSE_ANIMATION_DELAY = 50 + function dialogClose() { if (flag.value === 'full') { nextTick(() => { customStyle1.value.right = '8px' customStyle2.value.opacity = 0 - }, 50) + }, CLOSE_ANIMATION_DELAY) } else { customStyle2.value.opacity = 0 } }packages/vue/src/dialog-box/src/mobile-first.vue (1)
34-34: Consider documenting the line-height changeThe line-height change from
leading-4toleading-5.5affects the vertical spacing of the header content.Consider adding a comment explaining why this specific line height was chosen for better maintainability.
packages/renderless/src/dialog-box/index.ts (1)
118-120: Consider using a constant for the data-tag selector.The selector string
[data-tag=${constants.DIALOG_BOX_DATA_TAG}]should be defined as a constant to maintain consistency and avoid string duplication.+ const DATA_TAG_SELECTOR = `[data-tag=${constants.DIALOG_BOX_DATA_TAG}]` - props.rightSlide && (el.querySelector(selector).style.left = '') + props.rightSlide && (el.querySelector(DATA_TAG_SELECTOR).style.left = '')🧰 Tools
🪛 Biome (1.9.4)
[error] 120-120: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
examples/sites/demos/apis/dialog-box.js (2)
249-263: Improve the Chinese description for custom-style property.The Chinese description "自定义实现双层抽屉" (Custom implementation of double-layer drawer) doesn't match the English description "Custom Styles" and seems too specific. Consider updating it to be more generic, like "自定义样式" (Custom styles).
264-278: Consider using boolean type for no-animation property.The
no-animationproperty would be more intuitive as a boolean type since it's a flag to enable/disable animation, rather than a string.- type: 'string', + type: 'boolean', - defaultValue: ``, + defaultValue: false,examples/sites/demos/apis/form.js (1)
222-227: LGTM! Enhanced validation options.Added support for "deep" validation mode in validate-on-rule-change property. The type and description have been properly updated to reflect this addition.
Consider adding an example demonstrating the usage of the new "deep" validation mode in the documentation.
packages/theme-saas/src/divider/index.less (2)
1-71: Consider consolidating divider styles across themes.The styles in this file appear to duplicate those in
packages/theme/src/divider/index.less. Consider creating a shared base style file to reduce maintenance overhead.
43-50: Improve vertical divider implementation.The vertical divider uses fixed dimensions (
width: 1pxandheight: 1em) which might not scale well with different font sizes. Consider using relative units or CSS custom properties for better flexibility.- @apply w-1; - height: 1em; + @apply w-[var(--tv-divider-vertical-width,1px)]; + height: var(--tv-divider-vertical-height,1em);packages/theme/src/divider/index.less (1)
Line range hint
52-61: Enhance text alignment flexibility.The text positioning could be more flexible by using CSS custom properties for padding and alignment.
&__text { position: absolute; font-weight: 500; - padding: 0 14px; + padding: 0 var(--tv-divider-text-padding, 14px); color: var(--tv-divider-text-color); background-color: var(--tv-divider-text-bg-color); line-height: normal; + text-align: var(--tv-divider-text-align, center); }packages/theme-saas/src/select-dropdown/index.less (1)
94-99: Simplify selector and use CSS custom property for margin.The nested selector could be simplified, and the hardcoded margin value should be configurable.
- &:first-child { - > .tiny-svg, - > span > .tiny-svg { - margin-top: 1px - } - } + &:first-child .tiny-svg { + margin-top: var(--tv-select-dropdown-first-item-margin, 1px); + }packages/theme-saas/src/modal/index.less (1)
8-8: Enhance browser compatibility for text wrapping.Consider adding fallbacks for better cross-browser support of word wrapping behavior.
- overflow-wrap: break-word; + overflow-wrap: break-word; /* Newer browsers */ + word-wrap: break-word; /* Legacy browsers */ + -ms-word-break: break-all; /* IE support */ + word-break: break-word; /* Modern browsers */Also applies to: 146-146
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (82)
examples/sites/demos/apis/dialog-box.js(1 hunks)examples/sites/demos/apis/divider.js(4 hunks)examples/sites/demos/apis/form.js(1 hunks)examples/sites/demos/mobile-first/app/dialog-box/double-dialog-height.vue(1 hunks)examples/sites/demos/mobile-first/app/dialog-box/webdoc/dialog-box.js(1 hunks)examples/sites/demos/mobile-first/app/divider/basic-usage.vue(1 hunks)examples/sites/demos/mobile-first/app/divider/color.vue(1 hunks)examples/sites/demos/mobile-first/app/divider/content-position.vue(1 hunks)examples/sites/demos/mobile-first/app/divider/divider-margin.vue(1 hunks)examples/sites/demos/mobile-first/app/divider/offset.vue(1 hunks)examples/sites/demos/mobile-first/app/divider/status.vue(1 hunks)examples/sites/demos/mobile-first/app/divider/type.vue(1 hunks)examples/sites/demos/mobile-first/app/divider/vertical.vue(1 hunks)examples/sites/demos/mobile-first/app/divider/webdoc/divider.cn.md(1 hunks)examples/sites/demos/mobile-first/app/divider/webdoc/divider.en.md(1 hunks)examples/sites/demos/mobile-first/app/divider/webdoc/divider.js(1 hunks)examples/sites/demos/mobile-first/menus.js(1 hunks)examples/sites/demos/pc/app/date-picker/disabled-composition-api.vue(1 hunks)examples/sites/demos/pc/app/date-picker/disabled.vue(1 hunks)examples/sites/demos/pc/app/dialog-box/double-dialog-height-composition-api.vue(1 hunks)examples/sites/demos/pc/app/dialog-box/double-dialog-height.vue(1 hunks)examples/sites/demos/pc/app/dialog-box/webdoc/dialog-box.js(1 hunks)examples/sites/demos/pc/app/divider/basic-usage.spec.ts(0 hunks)examples/sites/demos/pc/app/divider/content-position-composition-api.vue(1 hunks)examples/sites/demos/pc/app/divider/content-position.vue(1 hunks)examples/sites/demos/pc/app/divider/contentPosition-composition-api.vue(0 hunks)examples/sites/demos/pc/app/divider/contentPosition.vue(0 hunks)examples/sites/demos/pc/app/divider/custom-style-composition-api.vue(1 hunks)examples/sites/demos/pc/app/divider/custom-style.vue(1 hunks)examples/sites/demos/pc/app/divider/customStyle-composition-api.vue(0 hunks)examples/sites/demos/pc/app/divider/customStyle.vue(0 hunks)examples/sites/demos/pc/app/divider/direction-composition-api.vue(1 hunks)examples/sites/demos/pc/app/divider/divider-type-composition-api.vue(1 hunks)examples/sites/demos/pc/app/divider/divider-type.vue(1 hunks)examples/sites/demos/pc/app/divider/status-composition-api.vue(1 hunks)examples/sites/demos/pc/app/divider/status.vue(1 hunks)examples/sites/demos/pc/app/divider/webdoc/divider.js(4 hunks)internals/cli/src/commands/build/build-ui.ts(2 hunks)internals/cli/src/shared/config.ts(2 hunks)packages/mobile/package.json(1 hunks)packages/mobile/utils/package.json(1 hunks)packages/mobile/vue-common/src/adapter/vue3/index.ts(1 hunks)packages/modules.json(1 hunks)packages/renderless/package.json(1 hunks)packages/renderless/src/dialog-box/index.ts(2 hunks)packages/renderless/src/divider/index.ts(1 hunks)packages/renderless/src/divider/vue.ts(1 hunks)packages/renderless/src/dropdown/index.ts(3 hunks)packages/renderless/src/dropdown/vue.ts(2 hunks)packages/renderless/src/file-upload/index.ts(1 hunks)packages/renderless/src/form/vue.ts(1 hunks)packages/renderless/src/rich-text-editor/vue.ts(2 hunks)packages/renderless/src/select/index.ts(1 hunks)packages/renderless/src/tree-menu/index.ts(1 hunks)packages/renderless/src/tree-menu/vue.ts(3 hunks)packages/renderless/src/year-table/index.ts(2 hunks)packages/renderless/types/divider.type.ts(1 hunks)packages/renderless/types/dropdown.type.ts(1 hunks)packages/renderless/types/index.ts(1 hunks)packages/renderless/types/tree-menu.type.ts(3 hunks)packages/theme-mobile/src/modal/index.less(1 hunks)packages/theme-saas/src/divider/index.less(1 hunks)packages/theme-saas/src/modal/index.less(2 hunks)packages/theme-saas/src/rich-text/index.less(1 hunks)packages/theme-saas/src/select-dropdown/index.less(1 hunks)packages/theme/src/divider/index.less(2 hunks)packages/theme/src/divider/vars.less(1 hunks)packages/utils/src/crypt/core.ts(0 hunks)packages/vue-directive/src/highlight-query.ts(2 hunks)packages/vue-locale/src/lang/en.ts(1 hunks)packages/vue/src/dialog-box/src/index.ts(2 hunks)packages/vue/src/dialog-box/src/mobile-first.vue(3 hunks)packages/vue/src/dialog-box/src/pc.vue(3 hunks)packages/vue/src/divider/src/index.ts(1 hunks)packages/vue/src/divider/src/mobile-first.vue(1 hunks)packages/vue/src/divider/src/pc.vue(2 hunks)packages/vue/src/divider/src/token.ts(1 hunks)packages/vue/src/form/src/index.ts(1 hunks)packages/vue/src/grid/src/adapter/src/renderer.ts(1 hunks)packages/vue/src/grid/src/table/src/methods.ts(3 hunks)packages/vue/src/grid/src/table/src/strategy.ts(2 hunks)packages/vue/src/input/src/mobile-first.vue(2 hunks)
⛔ Files not processed due to max files limit (6)
- packages/vue/src/modal/src/mobile-first.vue
- packages/vue/src/query-builder/src/hooks/useRule.ts
- packages/vue/src/rich-text-editor/package.json
- packages/vue/src/upload-list/src/mobile-first.vue
- packages/vue/src/upload/src/mobile-first.vue
- tsconfig.vue3.json
💤 Files with no reviewable changes (6)
- examples/sites/demos/pc/app/divider/customStyle.vue
- examples/sites/demos/pc/app/divider/contentPosition-composition-api.vue
- examples/sites/demos/pc/app/divider/customStyle-composition-api.vue
- examples/sites/demos/pc/app/divider/basic-usage.spec.ts
- packages/utils/src/crypt/core.ts
- examples/sites/demos/pc/app/divider/contentPosition.vue
✅ Files skipped from review due to trivial changes (10)
- packages/renderless/package.json
- packages/mobile/utils/package.json
- packages/mobile/package.json
- examples/sites/demos/mobile-first/app/divider/webdoc/divider.en.md
- examples/sites/demos/mobile-first/app/divider/webdoc/divider.cn.md
- examples/sites/demos/pc/app/divider/divider-type-composition-api.vue
- packages/vue/src/divider/src/token.ts
- examples/sites/demos/pc/app/divider/direction-composition-api.vue
- examples/sites/demos/pc/app/divider/content-position-composition-api.vue
- internals/cli/src/commands/build/build-ui.ts
🧰 Additional context used
🪛 eslint
packages/renderless/src/divider/vue.ts
[error] 17-17: Type string trivially inferred from a string literal, remove type annotation.
(@typescript-eslint/no-inferrable-types)
[error] 18-18: Type string trivially inferred from a string literal, remove type annotation.
(@typescript-eslint/no-inferrable-types)
🪛 Biome (1.9.4)
packages/renderless/src/divider/index.ts
[error] 53-53: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/renderless/src/dialog-box/index.ts
[error] 120-120: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
packages/mobile/vue-common/src/adapter/vue3/index.ts
[error] 32-32: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 68-71: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 79-82: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 84-86: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 88-90: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 100-100: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 105-107: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 112-113: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 120-120: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 218-218: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 240-242: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 268-269: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 458-461: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
🔇 Additional comments (53)
packages/renderless/src/rich-text-editor/vue.ts (1)
2-11: LGTM! Import statements are well-organized.The imports are properly structured and match the API array entries.
packages/renderless/types/tree-menu.type.ts (1)
26-29: LGTM! Import statements are properly organized.The new imports are correctly added and maintain consistent ordering with existing imports.
packages/renderless/src/tree-menu/vue.ts (3)
38-39: LGTM! Imports are properly organized.The new imports are correctly added and maintain consistent ordering with existing imports.
74-75: LGTM! API methods are properly exposed.The new methods are correctly added to the api array, making them available to consumers of the component.
126-128: Verify proper initialization order of dependent methods.The initialization order of methods in Object.assign is important if there are dependencies between them. Ensure that
computedTreeStyledoesn't depend ongetTreesince it's initialized after it.examples/sites/demos/pc/app/divider/status-composition-api.vue (1)
11-13: LGTM! Clean composition API usageThe script setup syntax is properly implemented with correct import statement.
examples/sites/demos/mobile-first/app/divider/offset.vue (2)
3-5: Verify negative offset handlingThe component uses negative offset values (-14px). Please verify that the component properly handles negative offsets and consider adding validation if needed.
9-17: Consider migrating to Composition APISimilar to divider-margin.vue, consider migrating to the Composition API with
<script setup>for consistency.examples/sites/demos/pc/app/divider/content-position.vue (1)
4-4: Well-structured demonstration of content positioningGood examples showing different content positions (left/center/right) with both percentage and pixel-based offsets.
Also applies to: 6-6, 8-8
internals/cli/src/shared/config.ts (1)
14-15: Verify rich text editor peer dependenciesSpreading peer dependencies directly into externals could introduce unexpected dependencies. Please verify:
- The content of rich_text_editor_pkg.peerDependencies
- That all peer dependencies should indeed be treated as external
✅ Verification successful
Rich text editor peer dependencies verified and approved
All listed peer dependencies are appropriate editor-related packages with specific version constraints. Treating them as externals is the correct approach to prevent duplicate instances and allow proper version management by consuming applications.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check rich text editor peer dependencies # Display peer dependencies from package.json jq '.peerDependencies' packages/vue/src/rich-text-editor/package.json # Check if any of these dependencies are also listed in devDependencies or dependencies jq '[.dependencies, .devDependencies, .peerDependencies] | add | keys | group_by(.) | map(select(length > 1))' packages/vue/src/rich-text-editor/package.jsonLength of output: 1283
packages/vue/src/divider/src/pc.vue (1)
3-21: LGTM! Template changes enhance component flexibility.The template changes are well-structured:
- Proper conditional class binding for status
- Clean separation of styles using computed properties
- Appropriate conditional rendering for vertical/non-vertical modes
packages/renderless/types/divider.type.ts (1)
1-31: LGTM! Well-structured type definitions.The type definitions are comprehensive and follow TypeScript best practices:
- Clear interface segregation
- Proper use of ExtractPropTypes
- Good separation of concerns
packages/vue/src/divider/src/index.ts (1)
6-9: LGTM! Type definitions are clear and concise.The addition of 'dotted' to BorderStyleType and the export of $constants enhance the component's API.
packages/renderless/src/divider/index.ts (2)
3-11: LGTM! Well-structured style computation.The
computedRootStylefunction correctly handles height, margin, and fontSize properties with proper fallbacks to state values.
26-42: LGTM! Clear text positioning logic.The
computedTextStylefunction handles text positioning well with proper defaults.examples/sites/demos/pc/app/divider/webdoc/divider.js (4)
15-15: Improve file naming consistency.The file names have been standardized to use kebab-case, which aligns with Vue.js best practices.
Also applies to: 41-41, 55-55
24-26: Update property reference in documentation.The description now correctly references the
verticalproperty instead ofdirection, improving documentation accuracy.
37-37: Enhance documentation clarity.The description now includes both
text-positionandoffsetattributes, providing clearer guidance for positioning customization.Also applies to: 39-39
51-51: Comprehensive documentation of styling options.The description now covers all available styling attributes (
line-color,content-background-color,content-color,height,margin), improving developer understanding.Also applies to: 53-53
packages/vue/src/dialog-box/src/index.ts (1)
24-24: LGTM: Clear constant naming.The new constant
DIALOG_BOX_DATA_TAGfollows the established naming convention.packages/renderless/src/dropdown/vue.ts (2)
39-40: LGTM: Clear function exports.The new functions
clickOutsideandtoggleFocusare properly exported.
47-47: Proper cleanup implementation.The addition of
onBeforeUnmounthook ensures proper cleanup of resources.Also applies to: 98-98
examples/sites/demos/mobile-first/app/dialog-box/webdoc/dialog-box.js (1)
85-97: LGTM! Well-structured demo configuration.The new demo entry is properly configured with clear, localized descriptions that accurately reflect the component's functionality.
examples/sites/demos/mobile-first/app/divider/webdoc/divider.js (2)
1-4: LGTM! Well-structured configuration.The basic configuration is clean and follows the standard pattern for demo documentation.
4-110: Comprehensive demo coverage with clear documentation.The demos cover all essential divider functionalities:
- Basic usage
- Margin settings
- Color customization
- Text position and offset
- Border styles
- Status variations
- Vertical orientation
Each demo includes well-written descriptions in both Chinese and English.
packages/vue/src/dialog-box/src/pc.vue (3)
13-15: LGTM! Proper animation control implementation.The duration attribute is correctly bound to the noAnimation prop, providing smooth control over transition effects.
144-146: LGTM! Well-documented props addition.The new props enhance the component's flexibility:
- customStyle: Allows custom styling
- noAnimation: Controls animation behavior
40-41: Verify draggable attribute usage across the codebase.The new data attributes look good, but let's verify their usage.
✅ Verification successful
The draggable attribute implementation is correct and consistent
The data attribute is properly integrated with the dialog box component's drag functionality:
- Bound to the draggable prop with correct default value
- Used in conjunction with proper drag event handlers
- Implemented consistently across both PC and mobile-first versions
Note: While the implementation is correct, there's a TODO in the test file for draggable functionality testing.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check usage of dialog-box-draggable attribute rg -A 2 'data-dialog-box-draggable'Length of output: 238
Script:
#!/bin/bash # Check for draggable prop definition and related code ast-grep --pattern 'props: { $$$ draggable $$$ }' # Search for any JavaScript code using this attribute selector rg "data-dialog-box-draggable" # Search for test files related to dialog box fd "dialog.*test|dialog.*spec" --type f # Search for documentation files fd "README|readme|doc" --type f | rg -i "dialog"Length of output: 1021
Script:
#!/bin/bash # Get the content of pc.vue to check prop definition cat packages/vue/src/dialog-box/src/pc.vue # Search for draggable-related tests rg -A 3 "draggable" packages/vue/src/dialog-box/__tests__/dialog-box.test.tsx # Search for drag-related functionality rg -A 3 "drag" packages/vue/src/dialog-box/src/Length of output: 6899
examples/sites/demos/apis/divider.js (2)
13-19: LGTM! Enhanced border-style options.The border-style property now includes 'dotted' as an option, with proper documentation in both languages.
81-153: Well-structured new properties with proper versioning.The new properties are well-documented with:
- Clear descriptions in both languages
- Proper version tagging (3.21.0)
- Appropriate demo references
- Mobile-first support where applicable
examples/sites/demos/mobile-first/menus.js (1)
128-129: LGTM! Proper menu organization.The Divider component is correctly added to the 'Others' section with appropriate Chinese and English names.
packages/vue/src/dialog-box/src/mobile-first.vue (2)
4-9: LGTM: Animation control enhancementThe addition of conditional animation duration based on
noAnimationprop provides better control over dialog transitions.
179-181: Verify prop documentationNew props have been added to enhance dialog box customization:
maxHeight: Controls maximum heightcustomStyle: Allows custom stylingnoAnimation: Toggles animationLet's verify if these props are properly documented:
packages/renderless/src/year-table/index.ts (1)
31-31: Improved date handling consistencyThe changes improve consistency in date handling by properly creating Date objects for year comparisons. This is a good improvement as it:
- Ensures consistent behavior with the
disabledDatefunction- Prevents potential type mismatches between number and Date comparisons
Also applies to: 90-90
packages/renderless/types/index.ts (1)
85-85: LGTM: Added divider type exportThe addition of the divider type export maintains consistency with the module's export pattern.
packages/renderless/src/dropdown/index.ts (2)
Line range hint
180-184: LGTM: Improved focus handling APIThe
toggleFocusfunction is now exported, making it reusable and consistent with the module's API design.
197-199: Enhanced focus event handlingThe event handlers now use more semantic API methods (
toggleFocusOnTrue/toggleFocusOnFalse) for focus management.Let's verify the implementation of these new API methods:
Also applies to: 273-275
✅ Verification successful
Focus management implementation verified ✓
The new semantic API methods
toggleFocusOnTrueandtoggleFocusOnFalseare correctly implemented as wrappers around thetoggleFocusfunction, properly managing the component's focus state. The implementation is clean, well-typed, and includes proper cleanup.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for the implementation of the new focus API methods ast-grep --pattern 'toggleFocusOnTrue = $$$' ast-grep --pattern 'toggleFocusOnFalse = $$$'Length of output: 94
Script:
#!/bin/bash # Check the specific file for these methods cat packages/renderless/src/dropdown/index.ts # Also search with ripgrep for any references rg "toggleFocusOn(True|False)" -t ts --context 3Length of output: 12085
packages/vue/src/grid/src/adapter/src/renderer.ts (1)
216-218: LGTM! The key addition enhances row identification.The addition of the
keyconstant derived fromrow[$table.rowId]improves the rendering process by providing a unique identifier for each row in the grid.packages/renderless/src/dialog-box/index.ts (1)
59-62: LGTM! The custom style support enhances dialog box customization.The addition of custom style support through
props.customStyleprovides flexibility in styling dialog boxes while maintaining the base styles.examples/sites/demos/pc/app/dialog-box/webdoc/dialog-box.js (1)
167-179: LGTM! The demo documentation is clear and comprehensive.The new demo entry for the double-layer dialog box is well-documented with clear descriptions in both Chinese and English, explaining the two scenarios for right-side pop-up windows.
packages/vue/src/input/src/mobile-first.vue (1)
88-88: LGTM! Consistent disabled state styling.The class addition ensures consistent text color styling for disabled state across breakpoints.
packages/vue/src/grid/src/table/src/methods.ts (2)
1630-1633: LGTM! Code simplification.The code has been simplified by removing unnecessary parameter destructuring and directly accessing scrollXStore from the instance.
Line range hint
2075-2082: Verify the impact of restoreScollFlag.The addition of
restoreScollFlagcould affect scroll behavior in other parts of the codebase. Please ensure this flag is properly handled in all scroll-related code paths.✅ Verification successful
Flag usage is properly implemented and scoped ✅
The
restoreScollFlagis correctly handled in a clear lifecycle: set before scroll restoration, checked during column rendering, and reset afterward. Its impact is properly contained to managing scroll state during column updates.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usages of restoreScollFlag to verify proper handling rg "restoreScollFlag" -A 3Length of output: 905
packages/vue-locale/src/lang/en.ts (1)
197-197: LGTM! Improved date range separator visibility.Changed the date range separator from empty string to '-', making the range boundary more visually clear to users.
examples/sites/demos/pc/app/divider/custom-style-composition-api.vue (1)
1-14: LGTM! Well-structured demo componentThe component effectively demonstrates various customization options for the tiny-divider component. The examples are clear and well-documented with descriptive labels.
packages/theme-mobile/src/modal/index.less (1)
155-155: LGTM! Good UX improvementAdding
overflow: autoto modal body types improves content scrolling behavior when content exceeds the visible area.packages/vue/src/grid/src/table/src/strategy.ts (2)
207-210: LGTM! Improved type safety with direct tableVm parameterThe function signature change from
argstotableVmprovides better type safety and clearer parameter intent.
Line range hint
219-224: Verify the scroll restoration behaviorThe new
restoreScollFlagcondition affects column slicing behavior. Please ensure this change doesn't cause unexpected scroll position jumps.✅ Verification successful
Virtual scrolling implementation verified
The
restoreScollFlagis part of a well-implemented virtual scrolling system. It correctly handles column re-slicing during scroll position restoration, ensuring smooth scrolling behavior without unexpected jumps.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other uses of restoreScollFlag to understand the full impact rg "restoreScollFlag" --type tsLength of output: 365
Script:
#!/bin/bash # Get context around the flag usage in methods.ts rg "restoreScollFlag" -B 5 -A 5 --type ts # Look for scroll-related methods and properties rg "scroll" --type ts packages/vue/src/grid/src/table/src/Length of output: 27214
packages/theme-saas/src/rich-text/index.less (1)
100-100: LGTM! Better text wrapping behaviorAdding
word-break: break-wordimproves how long words are handled, preventing overflow and ensuring better text layout.packages/modules.json (1)
921-922: LGTM! Added mobile-first support to Divider componentThe Divider component now supports both PC and mobile-first modes, improving its versatility.
packages/renderless/src/select/index.ts (1)
575-579: LGTM! Well-documented bug fix.The added condition prevents the dropdown panel from failing to close when in filter mode. The comment clearly explains the purpose of this change.
packages/renderless/src/file-upload/index.ts (1)
1514-1514:⚠️ Potential issueBreaking Change: New mandatory parameter added to getDownloadFileInfo.
The function now requires a
docVersionparameter. This change could break existing code that calls this function without providing the document version.Let's verify the impact of this change:
packages/theme/src/divider/vars.less (1)
8-19: LGTM! Well-structured CSS variable additions.The new CSS custom properties enhance the divider component with:
- Status-based colors (success, warning, info, error)
- Default color and border width
- Clear documentation for each property
The changes follow the existing pattern and use consistent variable naming.
| let options = { ...defaultOptions, ...props.options } | ||
| options.extensions = [...new Set([...defaultOptions.extensions, ...(props.options.extensions || [])])] |
There was a problem hiding this comment.
Add null check for props.options.extensions.
The current implementation might throw an error if props.options is undefined. Consider adding a null check:
-let options = { ...defaultOptions, ...props.options }
-options.extensions = [...new Set([...defaultOptions.extensions, ...(props.options.extensions || [])])]
+let options = { ...defaultOptions, ...(props.options || {}) }
+options.extensions = [...new Set([...defaultOptions.extensions, ...(props.options?.extensions || [])])]📝 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.
| let options = { ...defaultOptions, ...props.options } | |
| options.extensions = [...new Set([...defaultOptions.extensions, ...(props.options.extensions || [])])] | |
| let options = { ...defaultOptions, ...(props.options || {}) } | |
| options.extensions = [...new Set([...defaultOptions.extensions, ...(props.options?.extensions || [])])] |
| @@ -15,7 +15,7 @@ function editNodes(el: HTMLElement, nodes: HTMLElement[], query: string) { | |||
| // 2、处理收集后的节点，字符串搜索性能优于正则替换 | |||
| nodes.forEach((node) => { | |||
| const content = node.textContent as string | |||
| const start = content.toLowerCase().indexOf(query.toLowerCase()) | |||
| const start = content.indexOf(query) | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider maintaining case-insensitive search as default behavior
The changes from case-insensitive to case-sensitive string matching (toLowerCase().indexOf() → indexOf() and toLowerCase().includes() → includes()) might negatively impact user experience. Search functionality typically defaults to case-insensitive matching to be more user-friendly.
Consider either:
- Reverting to case-insensitive matching:
- const start = content.indexOf(query)
+ const start = content.toLowerCase().indexOf(query.toLowerCase())
- return content.includes(query)
+ return content.toLowerCase().includes(query.toLowerCase())- Or better, add a configuration option to control case sensitivity:
-function edit(el: HTMLElement, query: string) {
+function edit(el: HTMLElement, query: string, options: { caseSensitive?: boolean } = {}) {
if (query) {
const matchNodes = walk(el, (node: HTMLElement) => {
if ((node.parentElement as HTMLElement).classList.contains('tiny-hl-query-node')) {
return false
}
const content = node.textContent || ''
- return content.includes(query)
+ return options.caseSensitive
+ ? content.includes(query)
+ : content.toLowerCase().includes(query.toLowerCase())
})
- editNodes(el, matchNodes, query)
+ editNodes(el, matchNodes, query, options)
}
}This would allow users to opt-in to case-sensitive matching while maintaining the more user-friendly default behavior.
Also applies to: 61-61
| <script> | ||
| import { Divider } from '@opentiny/vue' | ||
|
|
||
| export default { | ||
| components: { | ||
| TinyDivider: Divider | ||
| } | ||
| } | ||
| </script> |
There was a problem hiding this comment.
💡 Codebase verification
Add documentation and tests for the status prop
The status prop is implemented in the Divider component but lacks proper documentation and test coverage. Please:
- Document the status prop in both English and Chinese documentation files
- Add test cases to verify the status prop functionality
- Update type definitions to explicitly include the status prop
The status prop supports the following values: 'default', 'success', 'warning', 'error', 'info'
🔗 Analysis chain
Verify status prop documentation in Divider component API.
The implementation looks good. However, let's ensure that the new status prop is properly documented in the Divider component's API documentation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for status prop documentation in component files
rg -A 5 "status:" "src/divider"
# Check if there are any existing test cases for the status prop
rg -A 5 "status" "__tests__/divider"
Length of output: 174
Script:
#!/bin/bash
# Find divider-related files
echo "=== Divider-related files ==="
fd -t f -i divider
# Search for component definition and props
echo -e "\n=== Component definition and props ==="
ast-grep --pattern 'export default {
$$$
props: {
$$$
}
$$$
}'
Length of output: 1220
Script:
#!/bin/bash
# Check type definitions
echo "=== Type definitions ==="
cat packages/renderless/types/divider.type.ts
# Check documentation
echo -e "\n=== English documentation ==="
cat examples/sites/demos/pc/app/divider/webdoc/divider.en.md
echo -e "\n=== Chinese documentation ==="
cat examples/sites/demos/pc/app/divider/webdoc/divider.cn.md
# Check test cases
echo -e "\n=== Test cases ==="
cat packages/vue/src/divider/__test__/divider.test.tsx
Length of output: 3905
Script:
#!/bin/bash
# Find and check the divider props definition
echo "=== Divider Props Definition ==="
fd -t f -i "divider" -e vue -e ts -e js packages/vue/src/divider/src
# If found, check its contents
if [ -f "packages/vue/src/divider/src/index.ts" ]; then
echo -e "\n=== Props Content ==="
cat packages/vue/src/divider/src/index.ts
fi
Length of output: 1660
| export const getTree = | ||
| ({ vm }) => | ||
| () => { | ||
| return vm.$refs.tree | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for missing tree ref.
The getTree function directly accesses vm.$refs.tree without checking if it exists. This could lead to runtime errors if the ref is not yet mounted or is missing.
export const getTree =
({ vm }) =>
() => {
- return vm.$refs.tree
+ if (!vm.$refs.tree) {
+ console.warn('Tree ref is not available')
+ return null
+ }
+ return vm.$refs.tree
}📝 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.
| export const getTree = | |
| ({ vm }) => | |
| () => { | |
| return vm.$refs.tree | |
| } | |
| export const getTree = | |
| ({ vm }) => | |
| () => { | |
| if (!vm.$refs.tree) { | |
| console.warn('Tree ref is not available') | |
| return null | |
| } | |
| return vm.$refs.tree | |
| } |
| <tiny-divider color="#55ccd9"></tiny-divider> | ||
| <br /> | ||
| <tiny-divider color="#eb74df"> 分割线 </tiny-divider> |
There was a problem hiding this comment.
Ensure consistent component naming between registration and usage
You have registered the component as TinyDivider but are using <tiny-divider> in the template. Vue component names are case-sensitive when using string templates. To avoid potential issues, use the same case when registering and using components.
Apply this diff to register the component name in kebab-case:
export default {
components: {
- TinyDivider: Divider
+ 'tiny-divider': Divider
}
}Alternatively, you can change the template to use <TinyDivider>:
- <tiny-divider color="#55ccd9"></tiny-divider>
+ <TinyDivider color="#55ccd9"></TinyDivider>
...
- <tiny-divider color="#eb74df"> 分割线 </tiny-divider>
+ <TinyDivider color="#eb74df"> 分割线 </TinyDivider>Committable suggestion skipped: line range outside the PR's diff.
| setTimeout(() => { | ||
| this.customStyle2.right = '85px' | ||
| }, 10) | ||
|
|
||
| setTimeout(() => { | ||
| this.customStyle2.opacity = 1 | ||
| }, 100) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve animation handling.
Multiple timeouts with magic numbers can be hard to maintain and may cause memory leaks.
+ const ANIMATION_DELAYS = {
+ POSITION: 10,
+ OPACITY: 100
+ }
+
+ data() {
+ return {
+ ...
+ timeouts: []
+ }
+ },
+ beforeDestroy() {
+ // Cleanup timeouts
+ this.timeouts.forEach(clearTimeout)
+ },
methods: {
openDialog() {
- setTimeout(() => {
+ const positionTimeout = setTimeout(() => {
this.customStyle2.right = '85px'
}, 10)
+ this.timeouts.push(positionTimeout)
- setTimeout(() => {
+ const opacityTimeout = setTimeout(() => {
this.customStyle2.opacity = 1
- }, 100)
+ }, ANIMATION_DELAYS.OPACITY)
+ this.timeouts.push(opacityTimeout)
}
}Also applies to: 115-117
| const rightW = document.body.offsetWidth * 0.3 | ||
| this.customStyle1.right = -rightW + 80 + 'px' | ||
| this.customStyle2.right = rightW + 'px' |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace direct DOM manipulation with responsive design.
Using document.body.offsetWidth directly can cause issues on window resize. Consider using CSS variables or Vue's reactive system.
- const rightW = document.body.offsetWidth * 0.3
- this.customStyle1.right = -rightW + 80 + 'px'
- this.customStyle2.right = rightW + 'px'
+ const PARENT_WIDTH_PERCENTAGE = 30
+ const INDENT_OFFSET = 80
+
+ // Add window resize listener in mounted
+ const updateDialogPositions = () => {
+ const rightW = window.innerWidth * (PARENT_WIDTH_PERCENTAGE / 100)
+ this.customStyle1.right = `${-rightW + INDENT_OFFSET}px`
+ this.customStyle2.right = `${rightW}px`
+ }
+
+ // Call initially
+ updateDialogPositions()Committable suggestion skipped: line range outside the PR's diff.
| <tiny-button type="primary" @click="boxVisibility2 = false"> 确定 </tiny-button> | ||
| </template> | ||
| </tiny-dialog-box> | ||
| <template #footer> | ||
| <tiny-button @click="boxVisibility = false">取消</tiny-button> | ||
| <tiny-button type="primary" @click="openDialog"> 子弹窗 </tiny-button> | ||
| </template> | ||
| </tiny-dialog-box> | ||
| </div> | ||
| </template> | ||
|
|
||
| <script> | ||
| import { Button, DialogBox } from '@opentiny/vue' | ||
|
|
||
| export default { | ||
| components: { | ||
| TinyButton: Button, | ||
| TinyDialogBox: DialogBox | ||
| }, | ||
| data() { | ||
| return { | ||
| flag: 'auto', | ||
| boxVisibility: false, | ||
| customStyle1: {}, | ||
| customStyle2: {}, | ||
| boxVisibility2: false | ||
| } | ||
| }, | ||
| methods: { | ||
| changFn() { | ||
| this.boxVisibility = true | ||
| }, | ||
| changModdeFn() { | ||
| this.flag = this.flag === 'full' ? 'auto' : 'full' | ||
| }, | ||
| openDialog() { | ||
| if (this.flag === 'full') { | ||
| this.customStyle1 = { | ||
| right: '8px', | ||
| bottom: '8px', | ||
| top: '8px', | ||
| height: 'auto', | ||
| borderRadius: '8px', | ||
| transition: 'right 0.2s ease-in-out' | ||
| } | ||
| this.customStyle2 = { | ||
| bottom: '8px', | ||
| top: '8px', | ||
| height: 'auto', | ||
| borderRadius: '8px', | ||
| opacity: 0 | ||
| } | ||
| const rightW = document.body.offsetWidth * 0.3 | ||
| this.customStyle1.right = -rightW + 80 + 'px' | ||
| this.customStyle2.right = rightW + 'px' | ||
| this.customStyle2.transition = 'right 200ms ease-in-out opacity 100ms linear' | ||
| this.boxVisibility2 = true | ||
|
|
||
| setTimeout(() => { | ||
| this.customStyle2.right = '85px' | ||
| }, 10) | ||
|
|
||
| setTimeout(() => { | ||
| this.customStyle2.opacity = 1 | ||
| }, 100) | ||
| } else { | ||
| this.customStyle1 = { | ||
| right: '8px', | ||
| bottom: '8px', | ||
| top: '8px', | ||
| height: 'auto', | ||
| borderRadius: '8px', | ||
| transition: 'right 0.2s ease-in-out' | ||
| } | ||
| this.customStyle2 = { | ||
| bottom: '8px', | ||
| top: 'unset', | ||
| height: 'auto', | ||
| borderRadius: '8px', | ||
| opacity: 0 | ||
| } | ||
| const rightW = document.body.offsetWidth * 0.3 | ||
| this.customStyle2.right = rightW + 46 + 'px' | ||
| this.customStyle2.transition = 'opacity 100ms linear' | ||
| this.boxVisibility2 = true | ||
|
|
||
| setTimeout(() => { | ||
| this.customStyle2.opacity = 1 | ||
| }, 100) | ||
| } | ||
| }, | ||
| dialogClose() { | ||
| setTimeout(() => { | ||
| this.customStyle1.right = '8px' | ||
| this.customStyle2.opacity = 0 | ||
| }, 50) | ||
| } | ||
| } | ||
| } | ||
| </script> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider extracting shared code into a common component.
This file is identical to the mobile-first version. To avoid code duplication and maintain consistency, consider:
- Creating a shared base component
- Extending it for mobile and PC specific implementations if needed
Example structure:
+ // src/components/dialog-box/base-double-dialog.vue
+ // Extract current implementation here
+
+ // examples/sites/demos/mobile-first/app/dialog-box/double-dialog-height.vue
+ import BaseDoubleDialog from '@/components/dialog-box/base-double-dialog.vue'
+
+ export default {
+ extends: BaseDoubleDialog,
+ // Add mobile-specific overrides if needed
+ }
+
+ // examples/sites/demos/pc/app/dialog-box/double-dialog-height.vue
+ import BaseDoubleDialog from '@/components/dialog-box/base-double-dialog.vue'
+
+ export default {
+ extends: BaseDoubleDialog,
+ // Add PC-specific overrides if needed
+ }Committable suggestion skipped: line range outside the PR's diff.
| @@ -58,7 +58,7 @@ const pickerOptions = { | |||
| } | |||
| const yearPickerOptions = { | |||
| disabledDate(year) { | |||
| return year > 2025 || year < 2022 | |||
| return year.getTime() > new Date(2025, 0, 1, 0).getTime() || year.getTime() < new Date(2022, 0, 1, 0).getTime() | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve date comparison precision and add error handling.
The current implementation might have edge cases around timezone handling. Consider:
- Using UTC dates for consistent comparison
- Adding error handling for invalid dates
const yearPickerOptions = {
disabledDate(year) {
- return year.getTime() > new Date(2025, 0, 1, 0).getTime() || year.getTime() < new Date(2022, 0, 1, 0).getTime()
+ try {
+ const yearTime = year.getTime();
+ const minDate = Date.UTC(2022, 0, 1);
+ const maxDate = Date.UTC(2025, 0, 1);
+ return yearTime > maxDate || yearTime < minDate;
+ } catch (error) {
+ console.error('Invalid date comparison:', error);
+ return true;
+ }
+ }
}Committable suggestion skipped: line range outside the PR's diff.
| "DividerMobileFirst": { | ||
| "path": "vue/src/divider/src/mobile-first.vue", | ||
| "type": "template", | ||
| "exclude": false | ||
| }, |
There was a problem hiding this comment.
💡 Codebase verification
Path needs to be updated in modules.json
The mobile-first implementation exists and follows the project structure, but the path in modules.json needs to be corrected:
- Change
"vue/src/divider/src/mobile-first.vue"to"packages/vue/src/divider/src/mobile-first.vue"
🔗 Analysis chain
Verify the mobile-first implementation
The new DividerMobileFirst template follows the project structure. Let's verify its implementation:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the mobile-first template exists and has required exports
fd "mobile-first.vue" "vue/src/divider"
# Check for any related mobile-first test files
fd "mobile-first" "examples/sites/demos" --type f
Length of output: 248
Script:
#!/bin/bash
# Find all vue files related to divider
fd ".*\.vue$" --type f | grep -i "divider"
# Find any mobile-first.vue files in the entire repo
fd "mobile-first\.vue$" --type f
# Check the actual structure under vue directory
fd . "vue" --type d
Length of output: 6732
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
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
Based on the comprehensive review of changes, here are the concise release notes:
New Features
Improvements
Bug Fixes
Documentation