Skip to content

feat: add onItemsChange callback to PaginatedTable#3300

Merged
andrewzamojc merged 2 commits into
mainfrom
DT-3724
Apr 15, 2026
Merged

feat: add onItemsChange callback to PaginatedTable#3300
andrewzamojc merged 2 commits into
mainfrom
DT-3724

Conversation

@andrewzamojc

@andrewzamojc andrewzamojc commented Apr 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds an optional onItemsChange callback prop to the api-paginated PaginatedTable component
  • Fires whenever $store.visibleItems changes (initial fetch, page navigation, cached page, page size change)
  • Follows the existing callback pattern used by onError, onShiftUp, onShiftDown, onSpace
  • Enables consumers to react to visible table data changes without slot prop workarounds

Motivation: Cloud UI's billing invoices page (DT-3724) needs to sync a chart component with the table's visible data. The current approach uses a comma expression in the slot to extract visibleItems, which is fragile. This callback provides a clean, first-class way to observe item changes.

This table needs the prop, for this chart.

Screenshot 2026-04-13 at 3 28 30 PM

Test plan

I built ui and tested the new prop in cloud-ui locally. Works well.

  • Verify existing PaginatedTable behavior is unchanged when onItemsChange is not provided
  • Pass onItemsChange callback and verify it fires on initial load
  • Verify callback fires on forward/backward page navigation
  • Verify callback fires on cached page navigation (no new fetch)
  • Verify callback fires on page size change

Adds an optional onItemsChange callback prop that fires whenever
the visible items change (page navigation, fetch, page size change).
Enables consumers to react to table data changes without relying
on slot prop workarounds.
@vercel

vercel Bot commented Apr 13, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
holocene Ready Ready Preview, Comment Apr 15, 2026 3:11pm

Request Review

@temporal-cicd

temporal-cicd Bot commented Apr 13, 2026

Copy link
Copy Markdown
Contributor
Warnings
⚠️

📊 Strict Mode: 2 errors in 1 file (0.2% of 1059 total)

src/lib/holocene/table/paginated-table/api-paginated.svelte (2)
  • L101:20: Element implicitly has an 'any' type because expression of type 'string' can't be used to index type '{ items: T[]; nextPageToken: string; }'.
  • L117:20: Element implicitly has an 'any' type because expression of type 'string' can't be used to index type '{ items: T[]; nextPageToken: string; }'.

Generated by 🚫 dangerJS against ea35eaf

@andrewzamojc
andrewzamojc marked this pull request as ready for review April 13, 2026 20:04
@andrewzamojc
andrewzamojc requested a review from a team as a code owner April 13, 2026 20:04
@andrewzamojc
andrewzamojc requested a review from laurakwhit April 13, 2026 20:13
@andrewzamojc
andrewzamojc enabled auto-merge (squash) April 14, 2026 20:43
Comment on lines +187 to +189
$: if (onItemsChange && $store.visibleItems) {
onItemsChange($store.visibleItems);
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Seems like ideally this should be migrated to Svelte 5. That way we could have a $bindable visibleItems prop and let consumers react in their own $effect.

In the meantime, this currently depends on the full $store derived store and will get triggered every time any pagination state changes. Maybe we want to guard with a reference check here?

Suggested change
$: if (onItemsChange && $store.visibleItems) {
onItemsChange($store.visibleItems);
}
let previousItems;
$: if (onItemsChange && $store.visibleItems !== previousItems) {
previousItems = $store.visibleItems;
onItemsChange($store.visibleItems);
}

Also, thought it might be worth noting that this fires on the initial load. Not sure if that's expected behavior or not. If we wanted to prevent that we could add a check like so ⬇️

if (previousItems !== undefined) {                                                                                                                                                          
    onItemsChange($store.visibleItems);                                                                                                                                                     
}       

@andrewzamojc andrewzamojc Apr 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think the initial load one might be necessary to get the first set.

I'll add previous items. It doesn't seem right to fire when the reference hasn't changed 👍

Tracks previous items reference to avoid redundant callbacks when
the store re-evaluates but visibleItems hasn't actually changed.
Initial call still fires since previousItems starts as undefined.
@andrewzamojc
andrewzamojc merged commit 5c7aa86 into main Apr 15, 2026
16 checks passed
@andrewzamojc
andrewzamojc deleted the DT-3724 branch April 15, 2026 15:31
rossnelson added a commit that referenced this pull request Apr 20, 2026
Auto-generated version bump from 2.48.3 to 2.48.4

Specific version: 2.48.4

Changes included:
- [`7cf1be53`](7cf1be5) Adjust breadcrumb spacing and min-height (#3304)
- [`c9834815`](c983481) Add missing Nexus Operation events to event groups (#3309)
- [`5c7aa86c`](5c7aa86) feat: add onItemsChange callback to PaginatedTable (#3300)
- [`66d3cf68`](66d3cf6) Update pre-release badge to secondary in Workers (#3310)
- [`393e84de`](393e84d) Keep Pending Activities/Operations at the top of Timeline and Compact view (#3307)
- [`9f721347`](9f72134) Update error link in codec-server-error-banner (#3313)
- [`e698bda3`](e698bda) Add class prop to BottomNav component (#3314)
- [`55767fc9`](55767fc) Add overrides (#3317)
- [`256cd274`](256cd27) refactor(decode-payload): clean up decode-payload.ts API surface (#3302)
- [`04f8c251`](04f8c25) Make input height full so its easier to click into the input (#3318)
- [`b2231056`](b223105) Workflows Table Row Density (#3285)
- [`0b2ce4fd`](0b2ce4f) PR Review Notifications (#3322)
- [`84618196`](8461819) feat: serverless worker deployment CRUD (#3236)

Co-authored-by: rossnelson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants