Skip to content

Conversation

@nowgnuesLee
Copy link
Contributor

@nowgnuesLee nowgnuesLee commented Nov 13, 2025

resolves #4634 (FR-1678)

This PR adds an auto-adjustment feature to the BAITable pagination that automatically corrects the current page when it exceeds the maximum available page after data changes. This prevents users from being stuck on an invalid page when filtering or data updates reduce the total number of items.

Key changes:

  • Added autoAdjustCurrentPage option to pagination config (defaults to true)
  • Implemented logic to calculate maximum page and adjust current page when needed
  • Used useEffectEvent to handle the adjustment logic properly

Checklist: (if applicable)

  • Documentation
  • Minium required manager version
  • Specific setting for review (eg., KB link, endpoint or how to setup)
  • Minimum requirements to check during review
  • Test case(s) to demonstrate the difference of before/after

@github-actions github-actions bot added the size:M 30~100 LoC label Nov 13, 2025
Copy link
Contributor Author


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • flow:merge-queue - adds this PR to the back of the merge queue
  • flow:hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has required the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@nowgnuesLee nowgnuesLee marked this pull request as ready for review November 13, 2025 07:14
Copilot AI review requested due to automatic review settings November 13, 2025 07:14
@github-actions
Copy link

Coverage report for ./packages/backend.ai-ui

St.
Category Percentage Covered / Total
🔴 Statements 49.62% 132/266
🔴 Branches 29.96% 80/267
🔴 Functions 33.33% 20/60
🔴 Lines 51.95% 120/231

Test suite run success

55 tests passing in 3 suites.

Report generated by 🧪jest coverage report action from 6733508

Copilot finished reviewing on behalf of nowgnuesLee November 13, 2025 07:17
@nowgnuesLee nowgnuesLee marked this pull request as draft November 13, 2025 07:17
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds automatic page adjustment functionality to BAITable to prevent users from being stuck on invalid pages when data changes reduce the total number of items. The feature defaults to enabled but can be disabled via configuration.

Key changes:

  • Added autoAdjustCurrentPage option to pagination config (defaults to true)
  • Implemented automatic page correction when current page exceeds maximum available page
  • Used useEffectEvent hook to handle adjustment logic

interface BAITablePaginationConfig
extends Omit<TablePaginationConfig, 'position'> {
/** Additional content to display in the pagination area */
extraContent?: ReactNode;
Copy link

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

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

The new autoAdjustCurrentPage property is missing JSDoc documentation. Please add a comment describing what this option does, its default value, and when it should be disabled.

Suggested documentation:

/** Whether to automatically adjust the current page when it exceeds the maximum available page (defaults to true) */
autoAdjustCurrentPage?: boolean;
Suggested change
extraContent?: ReactNode;
extraContent?: ReactNode;
/**
* Whether to automatically adjust the current page when it exceeds the maximum available page (defaults to true)
*/

Copilot uses AI. Check for mistakes.
Comment on lines +348 to +354
}, [
tableProps.dataSource,
tableProps.pagination,
autoAdjustCurrentPage,
currentPage,
currentPageSize,
]);
Copy link

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

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

The useEffect dependency array includes adjustCurrentPage, but since adjustCurrentPage is created by useEffectEvent (which should have stable identity), it shouldn't be included in the dependency array. However, since useEffectEvent is not a real React API (see other comment), this needs to be fixed.

When you switch to useEventNotStable (as recommended), remove adjustCurrentPage from the dependency array since that hook returns a stable function reference:

useEffect(() => {
  if (tableProps.pagination && autoAdjustCurrentPage) {
    adjustCurrentPage();
  }
}, [
  tableProps.dataSource,
  tableProps.pagination,
  autoAdjustCurrentPage,
  currentPage,
  currentPageSize,
  // adjustCurrentPage should be removed as useEventNotStable returns a stable reference
]);

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30~100 LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mismatch between displayed page and fetched data after project change

2 participants