Skip to content
This repository has been archived by the owner on May 24, 2024. It is now read-only.

[terra-paginator] Progressive-Paginator add support for unknown total pages #3157

Closed
kc4181 opened this issue Sep 12, 2020 · 6 comments · Fixed by #3190
Closed

[terra-paginator] Progressive-Paginator add support for unknown total pages #3157

kc4181 opened this issue Sep 12, 2020 · 6 comments · Fixed by #3190
Assignees
Milestone

Comments

@kc4181
Copy link

kc4181 commented Sep 12, 2020

Description

The standard Terra-Paginator currently has support for both when the total number of pages is known and when the total number of pages is not known.

The request is to similarly add support for when the total number of pages is not known to the Progressive-Paginator layout. Currently the progressive-paginator shows Page 1 of {total} and First <Previous Next> Last. When a total count is not known, the updates will remove the "of {total}" text and the "First" and "Last" buttons.

Example Screenshots

Above tiny breakpoint: Page 1 (Previous is dithered, Next is active)
Screen Shot 2020-09-29 at 7 22 31 AM

Above tiny breakpoint: Page 2+ (Previous and Next are both active)
Screen Shot 2020-09-29 at 7 22 55 AM

Tiny breakpoint: Page 1 (Previous is dithered, Next is active)
Screen Shot 2020-09-29 at 7 24 17 AM

Tiny breakpoint: Page 2+ (Previous and Next are both active)
Screen Shot 2020-09-29 at 7 24 38 AM

Related

@neilpfeiffer
Copy link
Member

neilpfeiffer commented Sep 29, 2020

A Tech Design needs to be completed first in order to determine the best approach. The current Progressive-Paginator has four props (onPageChange, selectedPage, totalCount, and itemCountPerPage) which are all are required. In order to create a version where totalCount is not known, a new subcomponent might need to be made, otherwise will need to determine if will be a Major Version Bump or not if existing subcomponent can be modified to handle the API for both.

@supreethmr
Copy link
Contributor

Need some inputs on choosing better approach to make Progressive-Paginator to support unknown page counts. As @neilpfeiffer mentioned we can create new subcomponent OR we can edit existing component by updating totalCount prop from required to optional. So here are the my findings on both approach:

  1. Creating New sub-component :

Tech Design :

Props for InfiniteProgressivePaginator:

Prop Type Default Required Description
onChange function none required Function to be executed when a navigation element is selected.
selectedPage number none optional The active/selected page, Selects 1st page when no value is provided.

Props for ControlledInfiniteProgressivePaginator:

Prop Type Default Required Description
onChange function none required Function to be executed when a navigation element is selected.
selectedPage number none required The active/selected page

Design Changes

  • First and Last button will not be included in new sub-components as there will be no totalCount provided to identify the last page.
  • itemCountPerPage prop is removed since having just itemCountPerPage without totalCount would have no value in component.

Pros :

  • Creating new sub-component will keep new changes to be aligned with existing Paginator pattern of restricting components for single functionality ( like controlled and uncontrolled paginators ).
  • easy to maintain as sub-component will not interfere with existing progressive paginator and will keep its purpose limited to only when page count is unKnown

Cons :

  • Requires two new sub-components ( InfiniteProgressivePaginator and ControlledInfiniteProgressivePaginator ) to support the new functionality which can also be accomplished with minor changes to existing APIs.

Link to sub-component implementation

  1. Modifying Existing Component :
  • Existing Progressive and Controlled Progressive Paginators can be updated to support unknown page count by having below changes to props :

Props for ProgressivePaginator:

Prop Type Default Required Description
onChange function none required Function to be executed when a navigation element is selected.
selectedPage number none optional The active/selected page, Selects 1st page when no value is provided.
totalCount number none optional Total number of all items being paginated.
itemCountPerPage number none optional Total number of items per page.

Props for ControlledProgressivePaginator:

Prop Type Default Required Description
onChange function none required Function to be executed when a navigation element is selected.
selectedPage number none required The active/selected page.
totalCount number none optional Total number of all items being paginated.
itemCountPerPage number none optional Total number of items per page.

Design Changes

  • The First and Last navigation button can be removed when totalCount is not provided similar to paginator component by having check for this.props.totalCount && this.props.itemCountPerPage.
  • Page Index that will be displayed along with navigation button is always set to selectedPageIndex so that it remains updated as we navigate using previous and next button.
  • Since we are updating props to be optional from required. I feel this will remain passive for existing component and might not require MVB as well.

Pros :

  • Reduces overall component lists for terra-paginator. By Eliminating the need of adding new sub-components and updates existing ProgressivePaginator and ControlledProgressivePaginator to support unknown page count similar to Paginator and ControlledPaginator.
  • Minimal code changes compare to adding new sub-components.

Cons :

  • Changing the existing Props to be optional from required might conflict with it's initial intent of having all of them as required in progressive paginator ( I was not able to find any impacts ( on existing scenario ) after making props to optional from required ).

link to code changes in existing component

@dkschoonover
Copy link
Contributor

@supreethmr, I know for some of our services we don't know how many total items there are when we start loading items, but we do know when we reach the last page. We were hoping the component design could support having a disabled "Next" button once we know we have reached the last page. From what I can tell this isn't possible with the current design proposal, but I could definitely be missing it.

@supreethmr
Copy link
Contributor

@dkschoonover Is there an indicator for page to identify as last page...?? OR will there be any unique pointers in page that makes it to identify as last page...??

@dkschoonover
Copy link
Contributor

@supreethmr, yeah, usually our services have a last page indicator so we know to stop loading in new pages.

@supreethmr
Copy link
Contributor

supreethmr commented Nov 6, 2020

@supreethmr, yeah, usually our services have a last page indicator so we know to stop loading in new pages.

Oh okay, Well then I think once the last page is identified we can calculate and update the totalPageCount prop which will make to paginator disable next button on last page and also provides first and last buttons for navigation.

Since paginator has no interaction with the data that is being paginated. I don't see much options to support the disabling of next button when page count is not known in paginator as of now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
6 participants