Skip to content
This repository has been archived by the owner on Oct 17, 2020. It is now read-only.

[BUG] Handle edge cases for page control #658

Open
rohithbalaji123 opened this issue Apr 11, 2020 · 1 comment
Open

[BUG] Handle edge cases for page control #658

rohithbalaji123 opened this issue Apr 11, 2020 · 1 comment
Labels
bug Something isn't working p0 High priority React Issue requires knowledge of React framework

Comments

@rohithbalaji123
Copy link
Member

Describe the bug
The currentPageIdx in PageControl component is not updated when totalPages prop is updated from the parent component. There is a possibility of a totalPages decreasing and if currentPageIdx is greater than the newly passed totalPages prop, it causes unexpected behavior.

To Reproduce
This is not in production yet. To effective reproduce this, create the component in HomePage with like following,

<PageControl
   totalPages={this.state.totalPages!}
   onPageChanged={this.onPageChanged}
/>
onPageChanged = () => {
    if (Math.random() > 0.5) {
      this.setState({ totalPages: 10 });
    } else {
      this.setState({ totalPages: 5 });
   }
};

Expected behavior
The PageControl should automatically change it's state to currently available last page if total pages count becomes less than the current page index.

Screenshots
pcBug

Additional context
In context with our application, this case could arise when user chooses to delete the only available URL in the last page of UserShortLinksSection.

@rohithbalaji123 rohithbalaji123 added bug Something isn't working React Issue requires knowledge of React framework p0 High priority labels Apr 11, 2020
@magicoder10 magicoder10 changed the title [BUG] Handle cases where totalPages [BUG] Handle edge cases for page control Apr 11, 2020
@Coteh Coteh self-assigned this May 6, 2020
@Coteh Coteh removed their assignment May 7, 2020
@Coteh
Copy link
Collaborator

Coteh commented May 8, 2020

Please see #748. A better solution has been proposed by @byliuyang to use cursor-based pagination instead of the current setup which relies on a total number of pages value.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working p0 High priority React Issue requires knowledge of React framework
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants