feat(table): allow disabling column sorting through header interactions#3716
feat(table): allow disabling column sorting through header interactions#3716
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis PR adds a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Table Component
participant Watcher
participant Tabulator
User->>Table Component: Set sortableColumns prop
Table Component->>Watcher: @Watch detects prop change
Watcher->>Table Component: Calls updateSortableColumns()
Table Component->>Table Component: Calls reinitialize()
Table Component->>Table Component: Calls getColumnDefinitions()
Note over Table Component: For each column, sets<br/>headerSort = sortableColumns && (headerSort ?? true)
Table Component->>Tabulator: Update column definitions
Tabulator->>Tabulator: Apply sorting state to headers
Tabulator->>User: Headers now reflect new sortable state
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
@paulinewahle once this PR is merged, you can update your beautiful What's new page with release date and versions and so forth 😊 🙏
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
etc/lime-elements.api.mdis excluded by!etc/lime-elements.api.md
📒 Files selected for processing (2)
src/components/table/examples/table-sorting-disabled.tsx(3 hunks)src/components/table/table.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/wrap-multiple-jsx-elements-in-host-not-in-array.mdc)
When returning multiple JSX elements from the
rendermethod, never wrap them in an array literal. Instead, always wrap them in the special<Host>element. When there is already a single top-level element, it does not have to be wrapped in<Host>.
Files:
src/components/table/examples/table-sorting-disabled.tsxsrc/components/table/table.tsx
⚙️ CodeRabbit configuration file
**/*.tsx: Our.tsxfiles are typically using StencilJS, not React. When a developer wants to return multiple top-level JSX elements from therendermethod, they will sometimes wrap them in an array literal. In these cases, rather than recommending they addkeyproperties to the elements, recommend removing the hardcoded array literal. Recommend wrapping the elements in StencilJS's special<Host>element.
Files:
src/components/table/examples/table-sorting-disabled.tsxsrc/components/table/table.tsx
**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
**/*.{ts,tsx}: Imports from other files in the same module (lime-elements) must use relative paths. Using absolute paths for imports will cause the production build to fail.
Files:
src/components/table/examples/table-sorting-disabled.tsxsrc/components/table/table.tsx
**/*.{tsx,scss}
⚙️ CodeRabbit configuration file
**/*.{tsx,scss}: Almost all our components use shadow-DOM. Therefore, we have no need of BEM-style class names in our CSS.
Files:
src/components/table/examples/table-sorting-disabled.tsxsrc/components/table/table.tsx
src/components/**/examples/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
src/components/**/examples/**/*.{ts,tsx}: These files are an exception to the rule that all imports should use relative paths. When these example files import something that is publicly exported by lime-elements, the import should be made from@limetech/lime-elements. If they import something from another file inside theexamplefolder, the import should use a relative path.
Files:
src/components/table/examples/table-sorting-disabled.tsx
src/components/**/*.tsx
⚙️ CodeRabbit configuration file
src/components/**/*.tsx: When contributors add new props to existing components in the lime-elements repository, they should always add documentation examples that demonstrate the new prop's usage and explain how it works. This helps with user adoption, feature discoverability, and maintains comprehensive documentation.
Files:
src/components/table/examples/table-sorting-disabled.tsxsrc/components/table/table.tsx
🧠 Learnings (2)
📚 Learning: 2025-04-25T13:32:56.396Z
Learnt from: adrianschmidt
PR: Lundalogik/lime-elements#3529
File: src/components/lime-ai-avatar/lime-ai-avatar.tsx:30-38
Timestamp: 2025-04-25T13:32:56.396Z
Learning: For StencilJS components in lime-elements, prefer using the `<Host>` element to wrap child elements in the render method instead of returning an array of elements, which eliminates the need for key attributes and follows StencilJS best practices.
Applied to files:
src/components/table/examples/table-sorting-disabled.tsx
📚 Learning: 2025-04-25T13:32:56.396Z
Learnt from: adrianschmidt
PR: Lundalogik/lime-elements#3529
File: src/components/lime-ai-avatar/lime-ai-avatar.tsx:30-38
Timestamp: 2025-04-25T13:32:56.396Z
Learning: For StencilJS components in lime-elements, prefer using the `<Host>` element to wrap child elements in the render method instead of returning an array of elements. This follows the standard pattern in the codebase and eliminates the need for key attributes.
Applied to files:
src/components/table/examples/table-sorting-disabled.tsx
🧬 Code graph analysis (1)
src/components/table/examples/table-sorting-disabled.tsx (1)
src/components/table/table.types.ts (1)
Column(5-45)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Docs / Publish Docs
- GitHub Check: Build
- GitHub Check: Test
🔇 Additional comments (6)
src/components/table/table.tsx (2)
115-120: LGTM! Clear documentation and appropriate default value.The new
sortableColumnsprop is well-documented and correctly distinguishes between header interactions and programmatic sorting. The default value oftruemaintains backward compatibility.
557-563: LGTM! Correct conjunction logic for global and per-column sorting.The implementation correctly combines the global
sortableColumnsprop with individual column settings:
- When
sortableColumnsisfalse, all columns become non-sortable- When
sortableColumnsistrue, each column respects its ownheaderSortsetting (defaulting totrue)This matches the requirements and maintains programmatic sorting capability via the
sortingprop.src/components/table/examples/table-sorting-disabled.tsx (4)
1-1: LGTM! Correct import for StencilJS pattern.The
Hostimport is correctly added to wrap multiple top-level elements in the render method, following StencilJS best practices.Based on learnings.
32-44: LGTM! Clear state initialization.The new
disableAllSortingstate and the updated column title clearly demonstrate the distinction between global sorting control and per-column control.
47-65: LGTM! Excellent example demonstrating the new feature.The example correctly demonstrates:
- Global sorting control via
sortableColumns={!this.disableAllSorting}- Per-column sorting control via
headerSort: falseon the Reference Person column- User-friendly checkbox with clear labeling
The logic is correct: when the checkbox is checked,
disableAllSortingbecomestrue, makingsortableColumnsevaluate tofalse, thereby disabling sorting.As per coding guidelines.
67-70: LGTM! Clean event handler.The handler correctly stops propagation and updates the state from the event detail.
359712e to
3a3c28d
Compare
|
🎉 This PR is included in version 38.29.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
fix #3715
Summary by CodeRabbit
Review:
Browsers tested:
(Check any that applies, it's ok to leave boxes unchecked if testing something didn't seem relevant.)
Windows:
Linux:
macOS:
Mobile: