Table scroll#123
Conversation
Signed-off-by: Sobyt483 <andrianingomel@gmail.com>
Signed-off-by: Sobyt483 <andrianingomel@gmail.com>
Signed-off-by: Sobyt483 <andrianingomel@gmail.com>
📝 WalkthroughWalkthroughThis PR adds configurable pagination growth modes and height styling to ChangesPagination Growth Mode & Height Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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.
Actionable comments posted: 3
🧹 Nitpick comments (1)
projects/ngx/declarative-ui/table/declarative-table/declarative-table.component.spec.ts (1)
17-17: 💤 Low valueRemove redundant
| undefinedfrom optional property.The
?operator already makes the property optional (can beundefined), so explicitly adding| undefinedto the type union is redundant.♻️ Simplified type
- growMode?: 'Scroll' | 'Button' | undefined; + growMode?: 'Scroll' | 'Button';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@projects/ngx/declarative-ui/table/declarative-table/declarative-table.component.spec.ts` at line 17, The type declaration for the optional property growMode in the DeclarativeTable spec is redundant because the ? already permits undefined; update the declaration from "growMode?: 'Scroll' | 'Button' | undefined" to "growMode?: 'Scroll' | 'Button'" in the declarative-table.component.spec.ts (look for the growMode property) so the union no longer explicitly includes undefined.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@projects/ngx/declarative-ui/stories/declarative-table.stories.ts`:
- Around line 118-120: The onPageSizeChange handler currently types its
parameter as CustomEvent<number> and uses limit.detail, which is wrong because
DeclarativeTable emits the number directly; change the parameter signature of
onPageSizeChange to accept a plain number (e.g., limit: number) and update the
body to use the number directly (set this.paginationLimit = limit and slice
this.resources using that value) so it no longer accesses .detail; update any
related references to onPageSizeChange to match the new parameter type if
necessary.
In
`@projects/ngx/declarative-ui/table-card/declarative-table-card.component.spec.ts`:
- Around line 791-814: Replace the unsafe (component as any) casts in the three
specs by defining a narrow test accessor type for the component that exposes the
tableConfig() signal (e.g., an interface or type alias like TestAccessor {
tableConfig(): ReturnType<DeclarativeTableCardComponent['tableConfig']> }) and
use that type when retrieving the component from setup (const { component } =
setup(...) as unknown as TestAccessor) or by creating a small helper
getTestAccessor(component) that returns the typed accessor; update the three
assertions to call tableConfig() through that typed accessor so the inline
ESLint disables and any casts are removed while keeping the same expectations
for growMode, height and loadMoreButtonText.
In
`@projects/ngx/declarative-ui/table/declarative-table/declarative-table.component.ts`:
- Around line 50-51: The inputs growMode and loadMoreButtonText currently
include | undefined which allows callers to explicitly pass undefined and
override the provided defaults; remove the | undefined from their generic types
so the defaults are enforced (change input<'Scroll' | 'Button' |
undefined>('Button') to input<'Scroll' | 'Button'>('Button') and input<string |
undefined>('Load More') to input<string>('Load More')), update any related type
annotations or usages referencing growMode or loadMoreButtonText if needed to
match the non-undefined types, and run tests/compile to ensure no downstream
type conflicts.
---
Nitpick comments:
In
`@projects/ngx/declarative-ui/table/declarative-table/declarative-table.component.spec.ts`:
- Line 17: The type declaration for the optional property growMode in the
DeclarativeTable spec is redundant because the ? already permits undefined;
update the declaration from "growMode?: 'Scroll' | 'Button' | undefined" to
"growMode?: 'Scroll' | 'Button'" in the declarative-table.component.spec.ts
(look for the growMode property) so the union no longer explicitly includes
undefined.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1cb438bc-d528-497c-a5cc-3f80f0cdfc56
📒 Files selected for processing (9)
docs/declarative-table-card.mddocs/declarative-table.mdprojects/ngx/declarative-ui/stories/declarative-table.stories.tsprojects/ngx/declarative-ui/table-card/declarative-table-card.component.htmlprojects/ngx/declarative-ui/table-card/declarative-table-card.component.spec.tsprojects/ngx/declarative-ui/table-card/models/configs.tsprojects/ngx/declarative-ui/table/declarative-table/declarative-table.component.htmlprojects/ngx/declarative-ui/table/declarative-table/declarative-table.component.spec.tsprojects/ngx/declarative-ui/table/declarative-table/declarative-table.component.ts
Summary by CodeRabbit
New Features
Documentation