Sync 3.18.0 to dev #2291
Sync 3.18.0 to dev #2291
Conversation
* fix(anchor): [anchor] fix anchor error when links is empty * fix(anchor): [anchor] fix anchor error when links is empty
* fix(grid): [grid] toolbar simple setting fix * fix(grid): [grid] toolbar simple setting not sync
…ct attr (#2283) * fix(dropdown-item): [dropdown] fix issues for v-auto-tip and add effect attr * fix(dropdown-item): [dropdown] Modify version for dropdown-item
…nc-3.18.0-to-dev-zzc
…nc-3.18.0-to-dev-zzc
WalkthroughThis pull request includes updates to several Changes
Possibly related PRs
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: 3
🧹 Outside diff range and nitpick comments (8)
packages/vue/src/grid/src/dragger/src/methods.ts (3)
68-68: LGTM! Consider adding JSDoc for the newtriggerparameter.The addition of the
triggerparameter enhances the flexibility of therowDropmethod by allowing customization of the drag handle. This is a good improvement.Consider adding JSDoc comments to document the purpose and expected value of the new
triggerparameter for better maintainability.
70-75: LGTM! Consider clarifying the comment abouttriggerusage.The new logic for assigning the
handlevariable improves the configurability of the drag handle. It provides a sensible default and includes a special case for whenrowHandleis 'index'.The comment on line 72 might be slightly misleading. Consider updating it to clarify that
triggeris still used as a fallback whenrowHandleis not 'index'. For example:-// 配置了 rowHandle === 'index'则忽略trigger +// 当 rowHandle === 'index' 时，使用特定的 handle，否则优先使用 trigger
77-77: LGTM! Consider destructuringcreateHandlerOnEndin the import statement.The simplification of the
rowDropOptionsobject by using thehandlevariable improves readability and maintainability. The use ofcreateHandlerOnEndfor theonEndproperty is a good practice.For consistency with other imports, consider destructuring
createHandlerOnEndin the import statement at the top of the file:-import { createHandlerOnEnd, getSortColumns } from './rowDrop' +import { createHandlerOnEnd, getSortColumns } from './rowDrop'packages/vue/src/form-item/src/pc.vue (1)
226-227: Enhanced class merging logic.The updated implementation improves class handling by explicitly constructing a merged class string and using
deduplicateCssClassto remove duplicates. This approach is more robust and less prone to errors compared to the previous implementation.Consider extracting this class merging logic into a separate utility function for reusability across the codebase. For example:
function mergeAndDeduplicateClasses(...classes: string[]): string { return deduplicateCssClass(classes.join(' ')); }Then, you can use it like this:
data.class = mergeAndDeduplicateClasses('tiny-tooltip', stringifyCssClass(data.class));This would make the code more maintainable and easier to test.
packages/vue/src/grid/src/table/src/methods.ts (4)
775-780: LGTM! Consider adding error handling.The addition of toolbar column configuration update is a good improvement for keeping the UI components in sync. However, it might be beneficial to add some error handling or logging in case the
toolbarVmis unexpectedlyundefined.Consider adding a simple check:
const toolbarVm = this.getVm('toolbar') // 合并更新toolbar的Column配置 if (toolbarVm) { toolbarVm.updateColumn(fullColumn) +} else { + console.warn('Toolbar view model not found. Unable to update column configuration.') }
Line range hint
782-791: LGTM! Consider adding a comment for clarity.The addition of
this.refreshColumn()is a good improvement to ensure the column state is fully updated after reloading customs. This change enhances the consistency of the table's state.Consider adding a brief comment to explain the purpose of the
refreshColumncall:let third = () => { + // Ensure column state is fully updated after reloading customs return this.refreshColumn().then(() => this.tableFullColumn.slice(0)) }
Line range hint
825-828: LGTM! Consider consistent error handling.The addition of toolbar column update is a good improvement for maintaining consistency between the table and toolbar states. This ensures that the toolbar always reflects the current state of the table columns.
For consistency with the earlier suggestion, consider adding error handling:
if (toolbarVm) { toolbarVm.updateColumn(fullColumn) +} else { + console.warn('Toolbar view model not found. Unable to update column configuration in watchColumn.') }
Line range hint
842-846: LGTM! Consider consistent error handling and performance optimization.The addition of toolbar setting update when column visibility changes is a good improvement for maintaining consistency between the table and toolbar states. This ensures that the toolbar always reflects the current visibility state of the table columns.
For consistency with earlier suggestions and to potentially optimize performance, consider the following changes:
const toolbarVm = this.getVm('toolbar') + +// Update toolbar settings only if toolbar exists and column visibility has changed if (toolbarVm) { - toolbarVm.updateSetting() + if (arguments.length) { + toolbarVm.updateSetting() + } +} else { + console.warn('Toolbar view model not found. Unable to update settings in handleVisibleColumn.') }This change ensures that the toolbar is only updated when the column visibility actually changes (i.e., when arguments are provided to the function), potentially reducing unnecessary updates.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (13)
- packages/renderless/package.json (2 hunks)
- packages/renderless/src/anchor/index.ts (1 hunks)
- packages/renderless/src/fluent-editor/options.ts (4 hunks)
- packages/theme/package.json (1 hunks)
- packages/vue/src/dropdown-item/package.json (2 hunks)
- packages/vue/src/form-item/package.json (1 hunks)
- packages/vue/src/form-item/src/pc.vue (2 hunks)
- packages/vue/src/grid-toolbar/package.json (1 hunks)
- packages/vue/src/grid-toolbar/src/index.ts (0 hunks)
- packages/vue/src/grid/package.json (2 hunks)
- packages/vue/src/grid/src/column/src/column.ts (1 hunks)
- packages/vue/src/grid/src/dragger/src/methods.ts (1 hunks)
- packages/vue/src/grid/src/table/src/methods.ts (1 hunks)
💤 Files with no reviewable changes (1)
- packages/vue/src/grid-toolbar/src/index.ts
✅ Files skipped from review due to trivial changes (2)
- packages/renderless/package.json
- packages/vue/src/dropdown-item/package.json
🧰 Additional context used
🔇 Additional comments (20)
packages/vue/src/form-item/package.json (5)
3-3: LGTM: Addition of "type" fieldThe addition of
"type": "module"is a good practice. It explicitly declares that this package uses ECMAScript modules, which is beneficial for modern JavaScript development.
4-4: LGTM: Version updateThe version update from 3.18.0 to 3.18.1 is consistent with the PR objective of syncing to a new version. This patch version increment suggests minor changes or bug fixes.
6-6: LGTM: Addition of "license" fieldThe addition of the "license" field with the MIT license is a positive change. It clearly communicates the terms under which the package can be used, modified, and distributed, which is crucial for open-source projects.
7-7: LGTM: Addition of "sideEffects" fieldSetting
"sideEffects": falseis an excellent optimization. It informs bundlers that this package has no side effects, enabling more aggressive tree-shaking and potentially reducing the final bundle size for applications using this package.
20-23: LGTM: Addition of devDependencies for testingThe addition of devDependencies for testing is a positive change. It suggests an improvement in the package's testing capabilities, which can lead to better code quality and reliability.
To ensure these new testing tools are being utilized effectively, please verify the existence and coverage of tests. Run the following script:
#!/bin/bash # Description: Check for the existence of test files and their coverage # Test: Search for test files echo "Searching for test files:" fd -e spec.ts -e spec.js -e test.ts -e test.js . packages/vue/src/form-item # Test: Check for vitest configuration echo "Checking for vitest configuration:" fd vitest.config.ts vitest.config.js . packages/vue/src/form-item # Note: Actual test coverage would typically be checked by running the tests, # which is beyond the scope of this script.packages/vue/src/grid/package.json (1)
3-3: "type" field moved to the top of the fileThe "type" field has been repositioned to the top of the file, right after the "name" field. While this change doesn't affect functionality, it might be part of a standardization effort for package.json files in the project.
Let's verify if this repositioning is consistent across other package.json files in the project:
#!/bin/bash # Description: Check the position of the "type" field in package.json files # Test: Check if "type" is consistently placed after "name" in package.json files echo "Checking position of 'type' field in package.json files:" rg --type json -A 3 '"name":' -g 'package.json' | rg 'type'packages/vue/src/grid-toolbar/package.json (5)
3-3: LGTM: Addition of "type": "module"The addition of
"type": "module"is a positive change. It explicitly declares that this package uses ECMAScript modules, which is consistent with modern JavaScript practices. This can lead to better tree-shaking and module resolution in projects consuming this package.
6-6: LGTM: Addition of "license": "MIT"The addition of the MIT license is a positive change. It clearly communicates the terms under which this package can be used, modified, and distributed. This is crucial for open-source projects and helps users understand their rights and obligations when using the package.
46-49: LGTM: Addition of devDependencies for testingThe addition of devDependencies for testing purposes is a positive change. This indicates an emphasis on code quality and maintainability through testing.
To ensure these testing tools are properly set up and utilized, please run the following script:
#!/bin/bash # Description: Check for test files and configuration # Test: Search for test files echo "Searching for test files:" fd -e spec.ts -e test.ts # Test: Check for Vitest configuration echo "Checking for Vitest configuration:" fd vitest.config.tsIf no test files or Vitest configuration are found, consider adding some basic tests to leverage these newly added devDependencies.
4-4: Verify version update consistencyThe version update to 3.18.1 is noted. This patch version increment suggests minor changes or bug fixes have been made.
Please ensure this version update is consistent across all relevant packages and documentation. You can use the following script to check for version consistency:
✅ Verification successful
Version update consistency verified
The version has been consistently updated to 3.18.1 across all relevant
package.jsonfiles, ensuring synchronization.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for version consistency across package.json files # Test: Search for version fields in package.json files rg --type json '"version":\s*"3\.18\.1"' -g 'package.json'Length of output: 254
15-44: Verify necessity of added dependenciesThe reorganization and addition of dependencies in the "dependencies" section is noted. This change expands the package's capabilities by including various UI components and utilities.
Please ensure that all added dependencies are necessary for this package. You can use the following script to check for the usage of these dependencies:
If any of these dependencies are not used directly in this package, consider moving them to peerDependencies or removing them to avoid unnecessary bloat.
✅ Verification successful
Please run the corrected script to verify the usage of added dependencies:
Please run the corrected script to verify the usage of added dependencies:
Dependencies are correctly used
The added dependencies are actively imported and utilized within the codebase, confirming their necessity for the package's functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for usage of added dependencies # Test: Search for imports of added dependencies rg --type typescript -e "from '@opentiny/vue-(col|common|dialog-box|grid|icon|layout|locale|modal|option|radio|radio-group|renderless|row|search|select|split|tab-item|tabs|theme|tooltip)'" -g '*.ts'Length of output: 59010
Script:
#!/bin/bash # Description: Check for usage of added dependencies # Test: Search for imports of added dependencies rg --type typescript "from '@opentiny/vue-(col|common|dialog-box|grid|icon|layout|locale|modal|option|radio|radio-group|renderless|row|search|select|split|tab-item|tabs|theme|tooltip)'" -g '*.ts'Length of output: 337
Script:
#!/bin/bash # Description: Check for usage of added dependencies # Test: Search for imports of added dependencies rg --type ts "from '@opentiny/vue-(col|common|dialog-box|grid|icon|layout|locale|modal|option|radio|radio-group|renderless|row|search|select|split|tab-item|tabs|theme|tooltip)'" -g '*.ts'Length of output: 76149
packages/vue/src/grid/src/column/src/column.ts (1)
194-196: Improved error handling infindTreecallbackThe changes in the
findTreecallback function enhance the robustness of the code by introducing a fallback mechanism fornode. This modification prevents potential runtime errors that could occur whennodeis undefined, which is particularly beneficial when dealing with JSX in default slots.(node) => { // Fallback to empty object if node is undefined const { componentOptions, type } = node || {} const componentName = (type && type.name) || (componentOptions && componentOptions.Ctor.extendOptions.name) // ... rest of the function }This change maintains the existing logic while adding a layer of protection against undefined values. It's a good practice to handle potential edge cases, making the code more resilient.
packages/renderless/src/anchor/index.ts (3)
127-129: Improved error handling insetChildOffsetTopThe addition of a null check for
props.linksenhances the robustness of the function. This change prevents potential errors that could occur ifprops.linksis undefined or empty, following best practices for defensive programming.
Line range hint
228-241: Enhanced scroll behavior and hash management inlinkClickThe updates to the
linkClickfunction improve its functionality in two key areas:
- The addition of
setCurrentHashenhances the management of the current link state.- The modified scroll behavior now correctly handles cases where the scroll container is not the document body.
These changes address potential edge cases and improve the overall reliability of the anchor link system.
Line range hint
1-241: Overall assessment of changesThe modifications to this file enhance the anchor link management system by improving error handling and refining scroll behavior. The changes are well-implemented and address potential edge cases, particularly in the
setChildOffsetTopandlinkClickfunctions. These improvements contribute to a more robust and reliable anchor system.However, to ensure comprehensive testing and maintain code quality, consider the following suggestions:
- Update or add unit tests to cover the new null check in
setChildOffsetTop.- Verify that the scroll behavior changes in
linkClickwork correctly across different browser environments.- Consider adding inline comments to explain the purpose of the new checks and modifications, especially for future maintainers.
To verify the impact of these changes, you can run the following script:
packages/vue/src/form-item/src/pc.vue (1)
186-189: Improved comment clarity.The updated comment provides better clarity about when the tooltip component is not used in the validation process. This change enhances code readability and maintainability.
packages/renderless/src/fluent-editor/options.ts (4)
3-38: LGTM: New font configuration constants addedThe addition of
fontFamilyConfig,fontSizeConfig, andlineHeightConfigconstants is a good improvement. These arrays provide a centralized and easily maintainable way to manage font-related options for the editor.
Line range hint
60-71: LGTM: Simplified toolbar function and improved font optionsThe changes to the
toolbarfunction are well-implemented:
- The function signature has been simplified by removing the
FluentEditorparameter.- Font-related options now use the newly added constants, which improves maintainability and potentially performance by avoiding dynamic imports.
These changes align well with the addition of the new font configuration constants.
210-210: LGTM: Simplified defaultOption functionThe changes to the
defaultOptionfunction are consistent with the updates made to thetoolbarfunction:
- The function signature has been simplified by removing the
FluentEditorparameter.- The
toolbaris now called without passingFluentEditor, which aligns with its updated signature.These changes maintain consistency across the file and simplify the overall structure.
Line range hint
3-210: Overall changes improve code structure and maintainabilityThe modifications in this file enhance the code in several ways:
- Introduction of centralized font configuration constants improves maintainability.
- Simplification of function signatures in
toolbaranddefaultOptionfunctions reduces complexity.- Replacement of dynamic imports with static arrays may improve performance.
These changes collectively contribute to a more robust and efficient implementation of the fluent editor options.
| "@opentiny/vue-icon": "workspace:~", | ||
| "@opentiny/vue-renderless": "workspace:~", | ||
| "@opentiny/vue-icon": "workspace:~" | ||
| "@opentiny/vue-tooltip": "workspace:~" |
There was a problem hiding this comment.
💡 Codebase verification
Unused Dependency Detected: @opentiny/vue-icon
The dependency @opentiny/vue-icon has been added to packages/vue/src/form-item/package.json but does not appear to be used in the codebase.
- Action Required: Consider removing
@opentiny/vue-iconif it's not being utilized to avoid unnecessary bloat.
🔗 Analysis chain
Verify the usage of the new dependency
The changes to the dependencies look good. The addition of @opentiny/vue-icon suggests new functionality or a refactor involving icons. The reordering of @opentiny/vue-tooltip improves readability.
Please verify that @opentiny/vue-icon is being used in the package. Run the following script to check for its usage:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for the usage of @opentiny/vue-icon in the package
# Test: Search for import statements or require calls for @opentiny/vue-icon
rg --type typescript --type vue 'from\s+[\'"]@opentiny/vue-icon[\'"]|require\([\'"]@opentiny/vue-icon[\'"]\)' packages/vue/src/form-item
Length of output: 449
| "name": "@opentiny/vue-grid", | ||
| "version": "3.18.0", | ||
| "type": "module", | ||
| "version": "3.18.5", |
There was a problem hiding this comment.
💡 Codebase verification
Version update incomplete across package.json files
The version was intended to be updated to 3.18.5, but multiple package.json files still reference version 3.18.0. Please ensure that all relevant package.json files are updated accordingly to maintain consistency.
🔗 Analysis chain
Version update from 3.18.0 to 3.18.5
The package version has been incremented from 3.18.0 to 3.18.5. This minor version bump suggests backward-compatible bug fixes or small features have been added.
To ensure consistency across the project, let's verify if this version update is reflected in other relevant files:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistency of version 3.18.5 across package.json files
# Test: Search for version 3.18.5 in all package.json files
echo "Checking for version 3.18.5 in package.json files:"
rg --type json '"version":\s*"3.18.5"' -g 'package.json'
# Test: Check if there are any remaining references to version 3.18.0
echo "Checking for any remaining references to version 3.18.0:"
rg --type json '"version":\s*"3.18.0"' -g 'package.json'
Length of output: 17992
| "name": "@opentiny/vue-theme", | ||
| "type": "module", | ||
| "version": "3.18.1", | ||
| "version": "3.18.3", |
There was a problem hiding this comment.
💡 Codebase verification
Version numbers are inconsistent across packages.
The packages/theme/package.json has been updated to version 3.18.3, while other related packages are on different versions such as 3.18.0, 3.18.4, and some vue packages are at 3.18.1 or 3.18.5. Please synchronize the version numbers across all packages to maintain consistency.
🔗 Analysis chain
LGTM: Version bump looks good.
The version update from 3.18.1 to 3.18.3 is consistent with the PR objective of synchronizing version 3.18.0 to the dev branch. This minor version bump suggests a patch or small feature update.
To ensure consistency across related packages, please run the following script:
This will help identify any potential version mismatches across different packages in the project.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify version consistency across related packages
# Test: Check for any inconsistencies in version numbers
echo "Checking version consistency across packages:"
fd -t f package.json | xargs grep -H '"version":' | sort
Length of output: 19317
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
Release Notes
New Features
mergeCustomColumnmethod to synchronize toolbar settings with table column configurations.Bug Fixes
Chores