Skip to content

[WC-3283]: Fix custom pagination#2116

Open
samuelreichert wants to merge 32 commits intomainfrom
fix/WC-3283-custom-pagination
Open

[WC-3283]: Fix custom pagination#2116
samuelreichert wants to merge 32 commits intomainfrom
fix/WC-3283-custom-pagination

Conversation

@samuelreichert
Copy link
Contributor

@samuelreichert samuelreichert commented Feb 26, 2026

Pull request type

New feature (non-breaking change which adds functionality)


Description

Adding the ability to show total count even when using virtual scrolling

@samuelreichert samuelreichert force-pushed the fix/WC-3283-custom-pagination branch 2 times, most recently from 853d08c to 2162f32 Compare February 27, 2026 12:35
@samuelreichert samuelreichert marked this pull request as ready for review February 27, 2026 12:37
@samuelreichert samuelreichert requested a review from a team as a code owner February 27, 2026 12:37
@samuelreichert samuelreichert force-pushed the fix/WC-3283-custom-pagination branch from 2162f32 to 6dd4e1c Compare March 2, 2026 14:52
@samuelreichert samuelreichert force-pushed the fix/WC-3283-custom-pagination branch from 6dd4e1c to 2097066 Compare March 4, 2026 14:56
@github-actions github-actions bot added the shared label Mar 4, 2026
@samuelreichert samuelreichert force-pushed the fix/WC-3283-custom-pagination branch 3 times, most recently from 25b265d to 092e537 Compare March 9, 2026 13:45
iobuhov
iobuhov previously approved these changes Mar 11, 2026
Copy link
Collaborator

@iobuhov iobuhov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Collaborator

@iobuhov iobuhov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To debug infinite loop issue use trace from mobx pacakge and trace what causes dynamicPage to change. Then try to break the loop.
Overall looks good, but need rewrite in dynamic pagination feature and need rewire in container. Please review generated code before committing.

iobuhov
iobuhov previously approved these changes Mar 17, 2026
Copy link
Collaborator

@iobuhov iobuhov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@samuelreichert samuelreichert requested review from iobuhov and r0b1n March 18, 2026 12:59
@samuelreichert samuelreichert force-pushed the fix/WC-3283-custom-pagination branch from 5ee963a to 135c797 Compare March 18, 2026 13:07
samuelreichert and others added 29 commits March 18, 2026 14:09
…n module and simplify tests

Extracts loadedRowsAtom into the shared pagination module so consumer
widgets no longer need to define it locally. Removes the redundant
computed() wrapper in DynamicPagination.feature.spec.ts boxAtom helper,
casting the observable.box directly with `as`.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… and drop local definition

Removes the local loadedRowsAtom wrapper (which was a redundant
computed() around itemCount) now that the shared module exports it.
Also cleans up the unused ComputedAtom and computed imports.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ture injection

The DynamicPaginationFeature constructor no longer takes a pageSize
parameter, so the extra GY.paging.pageSize token in the injected() call
caused a TS2554 build error. Also simplifies externalPageSize init to
use nullish coalescing instead of a > 0 guard.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…eValue<number>

IObservableValue<number> is a structural superset of ComputedAtom<number>,
so the intermediate NumberAtomBox wrapper and boxAtom helper are unnecessary.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@samuelreichert samuelreichert force-pushed the fix/WC-3283-custom-pagination branch from 135c797 to b953a32 Compare March 18, 2026 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants