-
Notifications
You must be signed in to change notification settings - Fork 715
fix: table col rezing fix #4406
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe changes involve enhancements to the logging functionality, including improved handling of search results, dynamic column width calculations, and the removal of a tabbed interface in the JSON preview component. Additionally, updates to column size management in the search results and performance optimizations in the table component have been implemented. A key renaming for local storage access was also made to ensure consistent naming conventions. Changes
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (2)
web/src/composables/useLogs.ts (1)
Line range hint
3056-3133: OptimizegetColumnWidthfunction for performance and error handling.The
getColumnWidthfunction uses a canvas context to measure text width, which can be performance-intensive if called frequently or with a large number of fields. Additionally, the error handling within the loop (lines 3118-3130) only logs to the console and does not provide a fallback width.Consider optimizing this function by caching measured widths or using a less resource-intensive method. Also, enhance error handling to ensure that a reasonable default width is used if text measurement fails.
web/src/utils/zincutils.ts (1)
287-287: Update all references to the old key namewrapcontent.The old key name
wrapcontentis still present in the codebase, specifically inweb/src/utils/zincutils.ts. Ensure that all instances are updated towrapContentto maintain consistency and avoid potential issues.
- File:
web/src/utils/zincutils.ts
- Line:
const wrapcontent: any = useLocalStorage("wrapContent", val, isDelete);Analysis chain
Approve the key renaming in
useLocalWrapContent.The renaming enhances consistency with JavaScript naming conventions. Ensure that all references to the old key name are updated throughout the codebase.
Run the following script to verify the key usage:
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to the old key name `wrapcontent` are updated. # Test: Search for the old key name. Expect: No occurrences of the old key name. rg --type typescript -A 5 $'wrapcontent'Length of output: 74
Script:
#!/bin/bash # Description: Verify all references to the old key name `wrapcontent` are updated. # Test: Search for the old key name in TypeScript files. Expect: No occurrences of the old key name. rg 'wrapcontent' --glob '*.ts' -A 5Length of output: 439
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- web/src/composables/useLogs.ts (5 hunks)
- web/src/plugins/logs/JsonPreview.vue (9 hunks)
- web/src/plugins/logs/SearchResult.vue (1 hunks)
- web/src/plugins/logs/TenstackTable.vue (4 hunks)
- web/src/utils/zincutils.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- web/src/plugins/logs/JsonPreview.vue
Additional comments not posted (4)
web/src/plugins/logs/TenstackTable.vue (3)
353-353: Import ofdebouncefunction is correct.The import statement is correctly placed and follows best practices for importing specific functions from libraries.
514-516:debouncedUpdatefunction is implemented correctly.The function correctly debounces the
update:columnSizesevent, which should help in reducing unnecessary re-renders and improve performance.Verify the integration of
debouncedUpdateacross the component to ensure it's being used effectively.Verification successful
debouncedUpdatefunction is correctly integrated and used.The function is invoked appropriately within the
TenstackTable.vuecomponent, ensuring that updates to column sizes are debounced effectively. This integration helps in optimizing performance by reducing unnecessary updates.
- File:
web/src/plugins/logs/TenstackTable.vue- Usage:
debouncedUpdate(newColSizes);Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `debouncedUpdate` across the component. rg --type vue -A 5 $'debouncedUpdate'Length of output: 64
Script:
#!/bin/bash # Description: Verify the usage of `debouncedUpdate` across the component by searching within .vue files. rg 'debouncedUpdate' --glob '*.vue' -A 5Length of output: 771
546-546: Adjustment inestimateSizefunction is noted.The change from 26 to 20 in the
estimateSizefunction may affect the layout or performance of the virtualized rows. It's important to verify this change in different environments and screen sizes to ensure it does not negatively impact the user experience.Verify the impact of this change on the layout and performance across different devices and screen sizes.
web/src/plugins/logs/SearchResult.vue (1)
251-262: Approve the updated column size handling logic.The implementation correctly merges previous and new column sizes, preserving user settings. Consider adding a comment explaining the merging logic for future maintainability.
| router.currentRoute.value.name == "logs" | ||
| ) { | ||
| queryReq.query.from = 0; | ||
| searchObj.meta.refreshHistogram = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using a more robust condition for setting refreshHistogram.
The current implementation sets refreshHistogram to true unconditionally when the refreshInterval is greater than zero and the current route is "logs". This might not always be the desired behavior, especially if there are conditions where the histogram should not be refreshed automatically.
Consider adding additional conditions or providing a way to configure this behavior based on user preferences or other application states.
| if ( | ||
| searchObj.data.stream.selectedFields.length == 0 || | ||
| !searchObj.data.queryResults?.hits?.length | ||
| ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clarify the logic for manual field removal and column visibility.
The conditions used to determine whether to manually remove fields or adjust column visibility are quite complex and could benefit from additional comments or refactoring for clarity. This is particularly important as the logic involves multiple conditions and affects the UI directly.
Consider simplifying the logic or breaking it down into smaller, more manageable functions that can be easily tested and maintained.
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> - **New Features** - Enhanced log search functionality with improved handling of search results and dynamic column width adjustments. - Simplified JSON preview experience by removing the tabbed interface for viewing JSON data. - **Bug Fixes** - Improved column size updates by preserving existing sizes during adjustments in search results. - **Performance Improvements** - Optimized column size updates with debouncing to enhance responsiveness. - Adjusted row virtualization settings for better rendering performance. - **Chores** - Updated naming conventions for consistency in local storage keys. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: omkark06 <[email protected]>
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> - **New Features** - Enhanced log search functionality with improved handling of search results and dynamic column width adjustments. - Simplified JSON preview experience by removing the tabbed interface for viewing JSON data. - **Bug Fixes** - Improved column size updates by preserving existing sizes during adjustments in search results. - **Performance Improvements** - Optimized column size updates with debouncing to enhance responsiveness. - Adjusted row virtualization settings for better rendering performance. - **Chores** - Updated naming conventions for consistency in local storage keys. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: omkark06 <[email protected]>
This reverts commit 7240361.
Summary by CodeRabbit
New Features
Bug Fixes
Performance Improvements
Chores