Skip to content

fix(pagination): optimize OdsPagination to prevent performance and memory issue #797

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ export class OdsPagination {
const isLeft = direction === 'left';
const tooltipLabel = isLeft ? this.labelTooltipPrevious : this.labelTooltipNext;
const arrowButtonId = isLeft ? this.leftArrowButtonId : this.rightArrowButtonId;
const isArrowDisabled = this.isDisabled || (isLeft && this.current === 1) || (!isLeft && this.current >= this.pageList.length);
const isArrowDisabled = this.isDisabled || (isLeft && this.current === 1) || (!isLeft && this.current >= this.actualTotalPages);

return (
<li class="ods-pagination__list__arrow">
Expand Down Expand Up @@ -278,8 +278,8 @@ export class OdsPagination {
{ renderEllipsisLeft && this.renderEllipsis('left') }

{
this.pageList.slice(1, this.pageList.length - 1).map((page, index) => {
return this.renderPage(index + 2, page.active);
this.pageList.slice(1, this.pageList.length - 1).map((page) => {
return this.renderPage(page.pageId, page.active);
})
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,44 +14,49 @@ function computeActualTotalPages(itemPerPage: number, totalItems: number | undef
}

function createPageList(totalPages: number, pageSelected: number): OdsPaginationPageList {
const pageList: OdsPaginationPageList = Array.from({ length: totalPages }, () => ({ active: false }));
const pageList: OdsPaginationPageList = [];

let startIndex = Math.max(pageSelected - DEFAULT_PAGE_OFFSET, 1);
const endIndex = Math.min(startIndex + ELLIPSIS_THRESHOLD, totalPages);

// If there are less than or equal to 5 pages, set all pages as active (all displayed).
if (totalPages <= (MAX_VISIBLE_ITEMS - DEFAULT_PAGE_OFFSET)) {
for (let i = 0; i < pageList.length; i++) {
pageList[i].active = true;
}
} else {
// If there are more than 5 pages, set only some of them as active (not all displayed).
if (totalPages - pageSelected < DEFAULT_PAGE_OFFSET) {
// If selected page is one of the last pages of a long list, show the last 5 pages.
startIndex = totalPages - ELLIPSIS_THRESHOLD;
for (let i = 0; i < totalPages; i++) {
pageList.push({ active: true, pageId: i + 1 });
}
return pageList;
}

for (let i = startIndex; i <= endIndex; i++) {
// If i is two pages away from the selected page, skip it if it is between 4 and totalPages-2.
if (i == pageSelected - DEFAULT_PAGE_OFFSET && pageSelected < totalPages - 1 && pageSelected > ELLIPSIS_THRESHOLD && pageSelected < totalPages - DEFAULT_PAGE_OFFSET) {
continue;
}
// If i is two pages away from the selected page, skip it if it is greater than 5.
if (i == pageSelected + DEFAULT_PAGE_OFFSET && pageSelected < totalPages - (DEFAULT_PAGE_OFFSET + MINIMUM_PAGE) && i > (MAX_VISIBLE_ITEMS - DEFAULT_PAGE_OFFSET)) {
continue;
}
pageList[i - 1].active = true;
}
// If there are more than 5 pages, set only some of them as active (not all displayed).
if (totalPages - pageSelected < DEFAULT_PAGE_OFFSET) {
// If selected page is one of the last pages of a long list, show the last 5 pages.
startIndex = totalPages - ELLIPSIS_THRESHOLD;
}

if (startIndex > 1) {
// If startIndex is not 1, show the first page as active.
pageList[0].active = true;
}
if (startIndex > 1) {
// If startIndex is not 1, show the first page as active.
pageList.push({ active: true, pageId: 1 });
}

if (endIndex < totalPages) {
// If endIndex is not equal to totalPages, show the last page as active.
pageList[totalPages - 1].active = true;
}
let adjustedStartIndex = startIndex;
let adjustedEndIndex = endIndex;

// get the start range of pages to show
if (pageSelected > ELLIPSIS_THRESHOLD && pageSelected < totalPages - DEFAULT_PAGE_OFFSET) {
adjustedStartIndex = Math.max(adjustedStartIndex, pageSelected - DEFAULT_PAGE_OFFSET + 1);
}
// get the end range of pages to show
if (pageSelected < totalPages - (DEFAULT_PAGE_OFFSET + MINIMUM_PAGE) && pageSelected > MAX_VISIBLE_ITEMS - DEFAULT_PAGE_OFFSET) {
adjustedEndIndex = Math.min(adjustedEndIndex, pageSelected + DEFAULT_PAGE_OFFSET - 1);
}

for (let i = adjustedStartIndex; i <= adjustedEndIndex; i++) {
pageList.push({ active: true, pageId: i });
}

if (endIndex < totalPages) {
// If endIndex is not equal to totalPages, show the last page as active.
pageList.push({ active: true, pageId: totalPages + 1 });
}

return pageList;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
interface OdsPaginationPageContent {
active: boolean;
pageId: number;
}

type OdsPaginationPageList = Array<OdsPaginationPageContent>;

export type {
OdsPaginationPageList,
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,21 @@ describe('ods-pagination navigation', () => {
expect(odsItemPerPageChange).toHaveReceivedEventDetail(expected);
expect(odsItemPerPageChange).toHaveReceivedEventTimes(1);
});

it('should change items per page and emit event even in large items scenario', async() => {
await setup('<ods-pagination default-current-page="2" total-items="6000000"></ods-pagination>');
const perPageSelectElement = await page.find('ods-pagination >>> ods-select');
const odsItemPerPageChange = await el.spyOnEvent('odsItemPerPageChange');
perPageSelectElement.setAttribute('value', '50');
await page.waitForChanges();

const expected: OdsPaginationItemPerPageChangedEventDetail = {
current: 50,
currentPage: 1,
totalPages: 120000,
};

expect(odsItemPerPageChange).toHaveReceivedEventDetail(expected);
expect(odsItemPerPageChange).toHaveReceivedEventTimes(1);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,9 @@ describe('ods-pagination rendering', () => {
});

it('should show correct number of steps for total pages', async() => {
await setup('<ods-pagination default-current-page="1" total-pages="7"></ods-pagination>');
await setup('<ods-pagination default-current-page="1" total-pages="10"></ods-pagination>');
const buttonList = await page.findAll('ods-pagination >>> li >>> ods-button');

expect(buttonList.length).toBe(10);
expect(buttonList.length).toBe(9);
});
});