fix: #696 #700
fix: #696 #700
Conversation
WalkthroughThe changes update the subproject commit reference in the documentation and adjust the header components. In the header file, the import for the column groups renderer is modified from a default to a named import, while an unused variable is removed in favor of directly passing column data. In the grouping column plugins, the Changes
Sequence Diagram(s)sequenceDiagram
participant Header as RevogrHeader
participant Group as ColumnGroupsRenderer
participant GroupHeader as HeaderGroupRenderer
Header->>Group: Pass computed `cols` prop
Group->>GroupHeader: Invoke group header rendering
GroupHeader-->>Group: Return rendered group header
Group-->>Header: Return all rendered headers
Suggested labels
Suggested reviewers
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 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: 0
🧹 Nitpick comments (1)
src/plugins/groupingColumn/columnGroupsRenderer.tsx (1)
24-36: Simplified and improved visibility detection logic.The new approach for determining visible groups:
- Extracts visible column indexes into an array
- Uses array operations to check if any column falls within a group's boundaries
- Makes the group visibility check more direct and efficient
This implementation directly supports the PR objective of grouping columns based on indexes rather than properties.
For even better readability, consider extracting the visibility check to a named function:
- // Check if any visible column is within this group's range - const isVisible = visibleIndexes.some(index => - index >= groupStartIndex && index <= groupEndIndex - ); + // Check if any visible column is within this group's range + const isVisible = isColumnGroupVisible(visibleIndexes, groupStartIndex, groupEndIndex);With an appropriate helper function defined at the top of the file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
docs(1 hunks)formats/number(1 hunks)packages/angular(1 hunks)packages/react(1 hunks)packages/svelte(1 hunks)packages/vue2(1 hunks)packages/vue3(1 hunks)src/components.d.ts(1 hunks)src/components/header/revogr-header.tsx(2 hunks)src/plugins/groupingColumn/columnGroupsRenderer.tsx(2 hunks)src/plugins/groupingColumn/headerGroupRenderer.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (7)
- packages/angular
- formats/number
- packages/vue2
- packages/react
- packages/vue3
- docs
- packages/svelte
🔇 Additional comments (8)
src/plugins/groupingColumn/headerGroupRenderer.tsx (1)
18-18: Export standardization improves import consistency.The change from a non-exported constant to a named export aligns with modern JavaScript/TypeScript best practices and makes imports more explicit in consuming files.
src/components.d.ts (1)
157-157:✅ Verification successful
Enhanced return type provides more detailed range information.
The updated return type for
getSelectedRange()now includes dimension information (AllDimensionType) along with the range data, which better supports the index-based column grouping approach described in the PR objectives.
🏁 Script executed:
#!/bin/bash # Check the implementation of getSelectedRange to ensure it returns the enhanced type # Look for the getSelectedRange method implementation rg "getSelectedRange\(\)" -A 10Length of output: 1705
Enhanced return type successfully integrates dimension info.
The updated signature in
src/components.d.tsnow accurately reflects thatgetSelectedRange()returns a Promise resolving to(RangeArea & AllDimensionType) | null. The implementation in bothsrc/components/revoGrid/viewport.service.tsandsrc/components/revoGrid/revo-grid.tsxconfirms this change—using a normalization (via?? null) to ensure a consistentnullwhen no range is selected, even though the internal return type may includeundefined.
- Confirmed that the returning value is enriched with dimension type details and supports the updated column grouping logic.
- Verified that the normalization in
revo-grid.tsxensures type consistency with the declaration incomponents.d.ts.src/components/header/revogr-header.tsx (2)
15-15: Import aligned with component's export style.Updated import statement correctly uses named import, reflecting the change in how ColumnGroupsRenderer is exported.
208-208: Simplified props passing with direct column data.The props have been updated to pass
colsdirectly rather than usingvisibleProps, which aligns with the modified component API and supports the index-based column grouping approach described in the PR objectives.src/plugins/groupingColumn/columnGroupsRenderer.tsx (4)
3-5: Import statements correctly updated to support new implementation.The changes to imports align with the component's new requirements and maintain consistency with the export style changes in HeaderGroupRenderer.
8-17: Props interface improved to use direct column data.The Props type definition has been updated to use direct column data (
cols: PositionItem[]) instead of the property mapping (visibleProps), which aligns with the PR objective of grouping columns based on indexes rather than properties.
19-21: Export style standardized for better module consistency.Changed from default export to named export, improving import/export consistency across the codebase.
57-59: Improved group boundary tracking with clear comments.The renamed variables and added comments make the logic easier to follow, showing how the start index advances for the next group.
|
| for (let i = 0; i < depth; i++) { | ||
| let groupStartIndex = 0; |
There was a problem hiding this comment.
this will not work, grouping not always start from 0 index, it can start from anywhere hence it's a flat group
Summary by CodeRabbit