feat: add reize components & demo #4862
Conversation
|
WalkthroughThe pull request introduces several changes across multiple files, primarily focusing on the addition of a new resizable component in a Vue.js application. Key modifications include updates to configuration files, the introduction of a new Vue component for resizing UI elements, and the addition of a new route for demonstrating this functionality. Additionally, changes to localization files and export statements enhance the module's interface and usability. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
packages/effects/common-ui/src/components/resize/resize.vueOops! Something went wrong! :( ESLint: 9.14.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/node_modules/@vben/eslint-config/dist/index.mjs' imported from /eslint.config.mjs 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: 5
🧹 Outside diff range and nitpick comments (7)
playground/src/views/demos/resize/basic.vue (2)
6-9: Add type annotations and extract magic numbers.Consider these improvements for better maintainability and type safety:
- Add explicit type annotations to refs
- Extract the magic number 200 into a named constant
+const INITIAL_POSITION = 200; + -const width = ref(200); -const height = ref(200); -const top = ref(200); -const left = ref(200); +const width = ref<number>(INITIAL_POSITION); +const height = ref<number>(INITIAL_POSITION); +const top = ref<number>(INITIAL_POSITION); +const left = ref<number>(INITIAL_POSITION);
25-25: Consider internationalizing the component text.The title and description contain mixed language text. Consider using a translation system for better internationalization support.
- <Page description="Resize组件基础示例" title="Resize组件"> + <Page :description="t('resize.basicExample')" :title="t('resize.component')">Don't forget to:
- Add the corresponding translations to your i18n system
- Import and use the translation function (t)
packages/effects/common-ui/src/components/resize/resize.vue (5)
825-831: Avoid variable shadowing by renaming parametersIn the
onMountedhook, the parameterdragHandlein theforEachcallback shadows the variabledragHandlefrom the outer scope. This can lead to confusion and potential bugs.Apply this diff to rename the parameter and avoid shadowing:
- [...($el?.querySelectorAll(dragHandle.value) || [])].forEach( - (dragHandle) => { - (dragHandle as HTMLElement).dataset.dragHandle = String( + [...($el?.querySelectorAll(dragHandle.value) || [])].forEach( + (handle) => { + (handle as HTMLElement).dataset.dragHandle = String( currentInstance?.uid, ); }, );
835-842: Prevent variable shadowing in 'dragCancel' event setupSimilarly, in the setup for
dragCancelelements, the parametercancelHandleshadows an outer variable. Renaming it will improve code clarity.Apply this diff to rename the parameter:
- [...($el?.querySelectorAll(dragCancel.value) || [])].forEach( - (cancelHandle) => { - (cancelHandle as HTMLElement).dataset.dragCancel = String( + [...($el?.querySelectorAll(dragCancel.value) || [])].forEach( + (handle) => { + (handle as HTMLElement).dataset.dragCancel = String( currentInstance?.uid, ); }, );
186-195: Limit event listeners to the component scopeAttaching event listeners to
document.documentElementcan cause unintended side effects across the entire document. It's better to attach them to the component's root element to confine the events within the component.Consider modifying the
addEventsandremoveEventsfunctions to use the component's root element:- document.documentElement.addEventListener(eventName, cb); + $el.addEventListener(eventName, cb); ... - document.documentElement.removeEventListener(eventName, cb); + $el.removeEventListener(eventName, cb);
924-944: Avoid simulating events in watchers for 'x' and 'y' propertiesIn the watchers for
xandy, the code simulates user interactions by callingbodyDown,bodyMove, andbodyUp. This approach can be fragile and hard to maintain.Consider updating the position directly or refactoring the logic to avoid simulating events:
- bodyDown({ pageX: left.value!, pageY: top.value! } as MouseEvent & - TouchEvent); - bodyMove({ x: delta, y: 0 }); - nextTick(() => { - bodyUp(); - }); + left.value = x.value; + // Update any dependent reactive properties or emit necessary events here
1068-1077: Enhance visibility of the active state outlineThe outline style for the active state uses a light dashed line which might not be easily noticeable.
Consider using a more prominent style to improve user experience:
- outline: 1px dashed #d6d6d6; + outline: 2px solid #4A90E2;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
.vscode/settings.json(1 hunks)cspell.json(1 hunks)packages/effects/common-ui/src/components/index.ts(1 hunks)packages/effects/common-ui/src/components/resize/index.ts(1 hunks)packages/effects/common-ui/src/components/resize/resize.vue(1 hunks)playground/src/router/routes/modules/demos.ts(1 hunks)playground/src/views/demos/resize/basic.vue(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/effects/common-ui/src/components/resize/index.ts
🔇 Additional comments (11)
packages/effects/common-ui/src/components/index.ts (1)
5-5: Verify the resize module exists.
The export statement looks good and follows the established pattern in the file. Let's verify the module exists.
✅ Verification successful
Export statement is valid and module exists with expected structure
The verification confirms:
- The resize directory exists at the expected location
- Both index.ts and resize.vue files are present
- The module correctly exports VResize component from resize.vue
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the resize module exists and exports expected components
# Test 1: Check if the resize directory exists
fd -t d "resize$" packages/effects/common-ui/src/components
# Test 2: Check for index.ts or resize.vue in the resize directory
fd -e ts -e vue "^(index|resize)\." packages/effects/common-ui/src/components/resize
# Test 3: Verify exports from the resize module
rg -A 5 "export.*from.*resize" packages/effects/common-ui/src/components/resize
Length of output: 507
playground/src/views/demos/resize/basic.vue (1)
1-4: Verify the common-ui package implementation.
The component correctly uses Vue 3's script setup syntax with TypeScript. However, let's verify the implementation of the imported components from '@vben/common-ui'.
✅ Verification successful
The imports from @vben/common-ui are correctly implemented and widely used across the codebase
Both VResize and Page components are properly implemented in the @vben/common-ui package:
VResizeis a feature-rich resizable component with drag and resize capabilitiesPageis a layout component that provides consistent page structure with header, content, and footer sections- Both components are actively used across multiple views in the playground and apps directories
- The imports follow Vue 3's module system best practices
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the VResize component implementation
# Expected: Find the VResize component definition and its props/events
# Search for VResize component definition
ast-grep --pattern 'export const VResize = defineComponent({
$$$
})'
# Search for Page component definition
ast-grep --pattern 'export const Page = defineComponent({
$$$
})'
Length of output: 142
Script:
#!/bin/bash
# Let's try a broader search to locate these components
# Search for files containing VResize
echo "=== Files containing VResize ==="
rg -l "VResize"
# Search for files containing Page component
echo -e "\n=== Files containing Page ==="
rg -l "Page"
# Search for package.json to verify @vben/common-ui
echo -e "\n=== Package dependencies ==="
find . -name "package.json" -exec sh -c 'echo "=== $1 ==="; cat "$1" | grep -A 5 -B 5 "@vben/common"' sh {} \;
# Try different component export patterns
echo -e "\n=== VResize component definitions ==="
rg "VResize.*=.*define" -A 5
echo -e "\n=== Page component definitions ==="
rg "Page.*=.*define" -A 5
Length of output: 61283
Script:
#!/bin/bash
# Let's try a different search strategy to locate these components
# Search for VResize component files
echo "=== VResize Component Files ==="
fd "resize" packages/effects/common-ui/src/components
fd "resize" -e ts -e vue packages/effects/common-ui/src
# Search for Page component files
echo -e "\n=== Page Component Files ==="
fd "page" packages/effects/common-ui/src/components
fd "page" -e ts -e vue packages/effects/common-ui/src
# Try to read the component files if found
echo -e "\n=== VResize Component Content ==="
cat packages/effects/common-ui/src/components/resize/index.ts || true
echo -e "\n=== Page Component Content ==="
cat packages/effects/common-ui/src/components/page/index.ts || true
cat packages/effects/common-ui/src/components/page/page.vue || true
Length of output: 3558
Script:
#!/bin/bash
# Let's check the VResize component implementation
echo "=== VResize Component Content ==="
cat packages/effects/common-ui/src/components/resize/resize.vue
# Let's also check if there are any other components using these imports
echo -e "\n=== Usage of VResize and Page components ==="
rg "import.*VResize.*from.*@vben/common-ui" -A 1
rg "import.*Page.*from.*@vben/common-ui" -A 1
Length of output: 31320
cspell.json (1)
25-26: LGTM! New resize-related terms added.
The addition of minh and minw terms aligns well with the PR's objective of adding resize components, as these likely represent min-height and min-width properties used in the implementation.
.vscode/settings.json (2)
225-225: LGTM: Trailing comma addition improves maintainability.
The addition of a trailing comma follows JSON formatting best practices and makes future additions cleaner.
226-226: Verify the decision to disable OXC linter.
While disabling OXC to avoid conflicts with ESLint is reasonable, please confirm if this was an intentional decision. The project already has comprehensive ESLint configuration (see editor.codeActionsOnSave and eslint.validate settings).
✅ Verification successful
Let me gather more information about the ESLint configuration and any related discussions about OXC.
Let me try one more verification to check the VSCode settings and ESLint configuration files directly:
Disabling OXC is appropriate given the existing linting setup
The VSCode settings show a comprehensive ESLint configuration is in place:
- ESLint is configured to run on save with
"source.fixAll.eslint": "explicit" - ESLint validation is set up for JavaScript and TypeScript files
- The project uses Prettier as the default formatter for all relevant file types
- There's no evidence of OXC being used elsewhere in the project dependencies or configurations
Disabling OXC avoids potential conflicts with the existing, well-established linting setup.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for OXC-related configurations and ESLint setup
# Test 1: Check if there are any OXC configurations in the project
echo "Checking for OXC configurations..."
rg -i "oxc" --type json
# Test 2: Verify ESLint configuration exists and is active
echo "Checking ESLint configuration..."
fd -e .eslintrc.json -e .eslintrc.js -e eslint.config.js -e eslint.config.mjs
Length of output: 257
Script:
#!/bin/bash
# Check for ESLint configuration in package.json and look for any OXC-related discussions in recent commits
echo "Checking package.json for ESLint config..."
cat package.json | jq '.eslintConfig, .devDependencies | select(. != null)'
echo -e "\nChecking recent commits for OXC discussions..."
git log --grep="oxc" -i --pretty=format:"%h %s" -n 5
echo -e "\nChecking VSCode settings for linting configuration..."
cat .vscode/settings.json | jq 'with_entries(select(.key | test("lint|format|oxc"; "i")))'
Length of output: 1643
Script:
#!/bin/bash
# Check VSCode settings and ESLint configuration
echo "VSCode settings related to linting:"
cat .vscode/settings.json | grep -A 2 -B 2 -i "lint\|format\|oxc"
echo -e "\nChecking for ESLint configuration files:"
find . -maxdepth 3 -type f -name ".eslintrc*"
echo -e "\nChecking workspace dependencies for OXC:"
find . -name "package.json" -exec grep -l "oxc" {} \;
Length of output: 2750
packages/effects/common-ui/src/components/resize/resize.vue (6)
849-864: Ensure consistent event handling for mouse and touch events
The bodyDown function handles both mouse and touch events but may not correctly handle cases where event properties differ between event types.
Please review the event handling logic to ensure that properties like ev.target, ev.stopPropagation(), and ev.preventDefault() are compatible and correctly used for both mouse and touch events across different devices.
708-711: Maintain reactivity when resetting 'limits' in 'bodyUp'
Similar to the previous comment, ensure that resetting limits in bodyUp doesn't break reactivity.
1079-1090: Ensure 'inactive' class is applied correctly to hide resize sticks
In the CSS, resize sticks are hidden when the .inactive class is present. Verify that the component correctly toggles between active and inactive classes based on the component's state.
514-602: Verify calculations in 'calcResizeLimits' function
The calcResizeLimits function contains complex calculations for resizing limits, especially when aspectRatio is true. Ensure that all mathematical operations are correct and handle edge cases.
Please double-check the formulas and consider adding unit tests for different scenarios to validate the calculations.
299-312:
Handle null values in 'sideCorrectionByLimit' function
The function sideCorrectionByLimit assumes limit.min and limit.max are numbers, but they can be null, which may cause comparison operations to fail.
Modify the function to handle null values safely:
- if (limit.min !== null && current < limit.min) {
+ if (limit.min !== null && current !== null && current < limit.min) {
value = limit.min;
} else if (limit.max !== null && limit.max < current) {
value = limit.max;
}Likely invalid or redundant comment.
610-615: Handle potential 'NaN' values in 'sizeStyle'
If width.value or height.value are undefined or null, the computed sizeStyle may result in NaN, causing rendering issues.
Run the following script to check for uninitialized width and height values:
|
still, need refactor. |
|
这个是组件，应该可以放到examples内，而不是demo文件夹 |
|
resize 组件超过1000行了，优化一下试试 |
|
css比较少，感觉可以改成tailwindcss，减少pacakges对scss的依赖 |
可以的，再封装一个hook还能更精简些 |
Clipchamp.mp4 |
当时写错了lang的类型，其实没有用到scss，只是css。用tailwind 是否激活状态比较难处理，还是先不改成tailwind了 |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (2)
playground/src/views/examples/resize/basic.vue (2)
6-9: Add type annotations and extract magic numberConsider these improvements for better type safety and maintainability:
- Add explicit type annotations to refs
- Extract the magic number 200 into a named constant
+const INITIAL_SIZE = 200; + -const width = ref(200); -const height = ref(200); -const top = ref(200); -const left = ref(200); +const width = ref<number>(INITIAL_SIZE); +const height = ref<number>(INITIAL_SIZE); +const top = ref<number>(INITIAL_SIZE); +const left = ref<number>(INITIAL_SIZE);
11-21: Consider type extraction and function memoizationThe resize handler could be optimized for better maintainability and performance:
- Extract the rectangle type definition
- Memoize the function since it's used in event handlers
+interface ResizeRect { + height: number; + left: number; + top: number; + width: number; +} + -const resize = (newRect: { - height: number; - left: number; - top: number; - width: number; -}) => { +const resize = computed(() => (newRect: ResizeRect) => { width.value = newRect.width; height.value = newRect.height; top.value = newRect.top; left.value = newRect.left; -}; +});
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
packages/effects/common-ui/src/components/resize/resize.vue(1 hunks)playground/src/locales/langs/zh-CN/examples.json(1 hunks)playground/src/router/routes/modules/examples.ts(1 hunks)playground/src/views/examples/resize/basic.vue(1 hunks)
🔇 Additional comments (4)
playground/src/views/examples/resize/basic.vue (1)
1-4: LGTM! Clean and focused imports.
The component setup is well-structured with appropriate TypeScript configuration and minimal necessary imports.
playground/src/locales/langs/zh-CN/examples.json (1)
12-14: LGTM! Verify the translation in UI context.
The Chinese translation "拖动调整" for the resize feature is semantically accurate and follows the established localization patterns. The JSON structure is properly maintained.
✅ Verification successful
Translation "拖动调整" is correctly used in the resize component
The Chinese translation "拖动调整" (drag adjustment) accurately describes the resize functionality as evidenced by:
- The translation key is properly used in the routes configuration for the resize demo
- The component implements drag-and-resize functionality with both dragging ("拖动") and adjustment/resizing ("调整") capabilities
- The translation aligns with the actual behavior of the VResize component which supports:
- Dragging the component body to move it
- Adjusting size by dragging resize handles
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the translation key usage in the codebase
# Expected: The translation key should be used in the resize demo component
# Search for usage of the translation key
rg -l "examples.resize.title"
# Search for the demo component to verify the context
fd -e vue resize
Length of output: 157
Script:
#!/bin/bash
# Check the usage context in the routes file
rg -A 5 "examples.resize.title" playground/src/router/routes/modules/examples.ts
# Check the resize component implementation
cat packages/effects/common-ui/src/components/resize/resize.vue
Length of output: 24990
packages/effects/common-ui/src/components/resize/resize.vue (2)
713-732: Confirm the necessity of 'active.value' check for resizing
In the stickDown method, the condition (!isResizable.value || !active.value) && !force might prevent resizing when active.value is false. Verify if the component should allow resizing when not active, or if the logic needs adjustment.
Consider whether the active state should influence the ability to resize. If resizing should be possible regardless of active, modify the condition accordingly.
475-499: 🛠️ Refactor suggestion
Refactor to avoid reassigning reactive objects
Directly reassigning reactive objects like limits.value can break reactivity. Instead, reset individual properties to maintain reactivity and avoid potential issues.
This issue has been previously noted. Please refer to the past review comments on lines 475-499 for guidance.
Also applies to: 690-711
| <template> | ||
| <Page description="Resize组件基础示例" title="Resize组件"> | ||
| <div class="m-4 bg-blue-500 p-48 text-xl"> | ||
| {{ | ||
| `width: ${width}px, height: ${height}px, top: ${top}px, left: ${left}px` | ||
| }} | ||
| </div> | ||
|
|
||
| <VResize | ||
| :h="200" | ||
| :is-active="true" | ||
| :w="200" | ||
| :x="200" | ||
| :y="200" | ||
| @dragging="resize" | ||
| @resizing="resize" | ||
| > | ||
| <div class="h-full w-full bg-red-500"></div> | ||
| </VResize> | ||
| </Page> | ||
| </template> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve i18n, reduce duplication, and enhance accessibility
Several improvements could be made to the template:
- Use i18n for the Chinese text
- Use reactive state values for VResize initial props
- Add ARIA attributes for better accessibility
<template>
- <Page description="Resize组件基础示例" title="Resize组件">
+ <Page :description="t('resize.basicExample')" :title="t('resize.component')">
<div class="m-4 bg-blue-500 p-48 text-xl">
{{
`width: ${width}px, height: ${height}px, top: ${top}px, left: ${left}px`
}}
</div>
<VResize
- :h="200"
- :w="200"
- :x="200"
- :y="200"
+ :h="height"
+ :w="width"
+ :x="left"
+ :y="top"
:is-active="true"
@dragging="resize"
@resizing="resize"
+ role="application"
+ aria-label="Resizable component"
>
<div class="h-full w-full bg-red-500"></div>
</VResize>
</Page>
</template>Committable suggestion skipped: line range outside the PR's diff.
| { | ||
| name: 'VueResizeDemo', | ||
| path: '/demos/resize/basic', | ||
| component: () => import('#/views/examples/resize/basic.vue'), | ||
| meta: { | ||
| icon: 'material-symbols:resize', | ||
| title: $t('examples.resize.title'), | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Align route path with established patterns
The route configuration has inconsistencies with the established patterns in the codebase:
- The path
/demos/resize/basicbreaks the established pattern of/examples/<feature>/<sub-feature>used by other routes. - The component is located in
examples/resize/basic.vuebut the route path usesdemos/.
Consider updating the route configuration to maintain consistency:
{
name: 'VueResizeDemo',
- path: '/demos/resize/basic',
+ path: '/examples/resize/basic',
component: () => import('#/views/examples/resize/basic.vue'),
meta: {
icon: 'material-symbols:resize',
title: $t('examples.resize.title'),
},
},Additionally, consider grouping resize-related routes under a parent route like other features (Form, VxeTable, etc.) to maintain a consistent hierarchy, especially if more resize-related routes will be added in the future.
📝 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.
| { | |
| name: 'VueResizeDemo', | |
| path: '/demos/resize/basic', | |
| component: () => import('#/views/examples/resize/basic.vue'), | |
| meta: { | |
| icon: 'material-symbols:resize', | |
| title: $t('examples.resize.title'), | |
| }, | |
| }, | |
| { | |
| name: 'VueResizeDemo', | |
| path: '/examples/resize/basic', | |
| component: () => import('#/views/examples/resize/basic.vue'), | |
| meta: { | |
| icon: 'material-symbols:resize', | |
| title: $t('examples.resize.title'), | |
| }, | |
| }, |
| const pageX = ev.pageX === undefined ? ev.touches![0]!.pageX : ev.pageX; | ||
| const pageY = ev.pageY === undefined ? ev.touches![0]!.pageY : ev.pageY; |
There was a problem hiding this comment.
Add checks for touch events to prevent runtime errors
When accessing ev.touches[0], there's a risk that ev.touches might be undefined or empty, leading to runtime errors. Ensure that ev.touches exists and has at least one touch point before accessing it.
Modify the code to include safety checks:
// Move function adjustments
const pageX = ev.pageX === undefined
- ? ev.touches![0]!.pageX
+ ? ev.touches && ev.touches[0] ? ev.touches[0].pageX : 0
: ev.pageX;
const pageY = ev.pageY === undefined
- ? ev.touches![0]!.pageY
+ ? ev.touches && ev.touches[0] ? ev.touches[0].pageY : 0
: ev.pageY;
// bodyDown function adjustments
const pointerX = ev.pageX === undefined
- ? ev.touches[0]!.pageX
+ ? ev.touches && ev.touches[0] ? ev.touches[0].pageX : 0
: ev.pageX;
const pointerY = ev.pageY === undefined
- ? ev.touches[0]!.pageY
+ ? ev.touches && ev.touches[0] ? ev.touches[0].pageY : 0
: ev.pageY;Also applies to: 894-895
| dragHandle.value && | ||
| (target! as HTMLElement).dataset.dragHandle !== | ||
| getCurrentInstance()?.uid.toString() | ||
| ) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Ensure 'ev.target' is an HTMLElement before accessing properties
In the conditional check, (target as HTMLElement).dataset.dragHandle, if ev.target is not an HTMLElement, accessing dataset may cause a runtime error. Add a type guard to ensure target is an HTMLElement.
Update the condition to include a type check:
if (
dragHandle.value &&
- (target! as HTMLElement).dataset.dragHandle !== getCurrentInstance()?.uid.toString()
+ target instanceof HTMLElement &&
+ target.dataset.dragHandle !== getCurrentInstance()?.uid.toString()
) {
return;
}📝 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.
| dragHandle.value && | |
| (target! as HTMLElement).dataset.dragHandle !== | |
| getCurrentInstance()?.uid.toString() | |
| ) { | |
| return; | |
| } | |
| dragHandle.value && | |
| target instanceof HTMLElement && | |
| target.dataset.dragHandle !== | |
| getCurrentInstance()?.uid.toString() | |
| ) { | |
| return; | |
| } |
| if (minw.value / minh.value > (aspectFactor.value as number)) { | ||
| minh.value = minw.value / (aspectFactor.value as number); | ||
| } else { | ||
| minw.value = ((aspectFactor.value as number) * minh.value) as number; | ||
| } | ||
| } |
There was a problem hiding this comment.
Avoid mutating props directly
In calcResizeLimits, the code assigns new values to minw.value and minh.value, which are props. Directly mutating props is an anti-pattern in Vue and can lead to unexpected behavior. Instead, use local reactive variables to hold modified values.
Apply this refactor to prevent prop mutation:
+const minWidth = ref(minw.value);
+const minHeight = ref(minh.value);
...
if (aspectRatio.value) {
if (minWidth.value / minHeight.value > aspectFactor.value!) {
- minh.value = minw.value / aspectFactor.value!;
+ minHeight.value = minWidth.value / aspectFactor.value!;
} else {
- minw.value = aspectFactor.value! * minh.value;
+ minWidth.value = aspectFactor.value! * minHeight.value;
}
}Committable suggestion skipped: line range outside the PR's diff.
| left.value) as number; | ||
| bottom.value = (parentHeight.value - | ||
| (h.value === 'auto' ? container.value!.scrollHeight : (h.value as number)) - | ||
| top.value) as number; |
There was a problem hiding this comment.
Check for 'container.value' before accessing properties
When calculating right.value and bottom.value, container.value might be undefined, leading to runtime errors when accessing scrollWidth or scrollHeight. Ensure that container.value is defined before using it.
Adjust the assignments with null checks:
right.value = parentWidth.value -
(w.value === 'auto'
- ? container.value!.scrollWidth
+ ? container.value ? container.value.scrollWidth : 0
: w.value as number) -
left.value;
bottom.value = parentHeight.value -
(h.value === 'auto'
- ? container.value!.scrollHeight
+ ? container.value ? container.value.scrollHeight : 0
: h.value as number) -
top.value;Committable suggestion skipped: line range outside the PR's diff.
Description
Type of change
Please delete options that are not relevant.
pnpm-lock.yamlunless you introduce a new test example.Checklist
pnpm run docs:devcommand.pnpm test.feat:,fix:,perf:,docs:, orchore:.Summary by CodeRabbit
Release Notes
New Features
VResize) for enhanced user interaction./demos/resize/basic.Localization
Bug Fixes
These updates improve the overall user experience by adding new interactive elements and enhancing localization support.