Skip to content

Commit 1dbe539

Browse files
committed
fix(ui-pagination): fix Pagination sometimes displaying non li elements in lists, fix key errors too
Closes: INSTUI-4422 TEST PLAN: check all examples in inspector, the pagination buttons should all be in lists. There should be no duplicate key warning in the console
1 parent 1993282 commit 1dbe539

File tree

1 file changed

+28
-16
lines changed
  • packages/ui-pagination/src/Pagination

1 file changed

+28
-16
lines changed

packages/ui-pagination/src/Pagination/index.tsx

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
* SOFTWARE.
2323
*/
2424
/** @jsx jsx */
25-
import React, { Component } from 'react'
25+
import React, { Component, ReactElement, ReactNode } from 'react'
2626

2727
import { View } from '@instructure/ui-view'
2828
import { testable } from '@instructure/ui-testable'
@@ -104,7 +104,7 @@ class Pagination extends Component<PaginationProps> {
104104
currentPage: 1,
105105
siblingCount: 1,
106106
boundaryCount: 1,
107-
ellipsis: <li style={{all: 'unset'}}>...</li>,
107+
ellipsis: '...',
108108
renderPageIndicator: (page: number) => page
109109
}
110110

@@ -317,7 +317,7 @@ class Pagination extends Component<PaginationProps> {
317317
renderPagesInInterval = (from: number, to: number, currentPage: number) => {
318318
if (to - from > 1000)
319319
throw new Error('Pagination: too many pages (more than 1000)')
320-
const pages = []
320+
const pages: ReactElement[] = []
321321
for (let i = from; i <= to; i++) {
322322
pages.push(
323323
<Pagination.Page
@@ -342,18 +342,26 @@ class Pagination extends Component<PaginationProps> {
342342
boundaryCount,
343343
variant
344344
} = this.props
345-
const pages: any = []
345+
const pages: ReactNode[] = []
346346
if (
347347
totalPageNumber! <= 2 * boundaryCount! ||
348348
totalPageNumber! <= 1 + siblingCount! + boundaryCount! ||
349349
variant === 'full'
350350
) {
351-
return this.renderPagesInInterval(1, totalPageNumber!, currentPage!)
351+
return (
352+
<ul css={this.props.styles?.pageIndicatorList}>
353+
{this.renderPagesInInterval(1, totalPageNumber!, currentPage!)}
354+
</ul>
355+
)
352356
}
353357

354358
if (currentPage! > boundaryCount! + siblingCount! + 1) {
355359
pages.push(this.renderPagesInInterval(1, boundaryCount!, currentPage!))
356-
pages.push(ellipsis)
360+
pages.push(
361+
<li key="ellipsis1" style={{ all: 'unset' }}>
362+
{ellipsis}
363+
</li>
364+
)
357365
if (
358366
currentPage! - siblingCount! >
359367
totalPageNumber! - boundaryCount! + 1
@@ -365,7 +373,7 @@ class Pagination extends Component<PaginationProps> {
365373
currentPage!
366374
)
367375
)
368-
return pages
376+
return <ul css={this.props.styles?.pageIndicatorList}>{pages}</ul>
369377
}
370378
pages.push(
371379
this.renderPagesInInterval(
@@ -392,7 +400,11 @@ class Pagination extends Component<PaginationProps> {
392400
currentPage!
393401
)
394402
)
395-
pages.push(ellipsis)
403+
pages.push(
404+
<li key="ellipsis2" style={{ all: 'unset' }}>
405+
{ellipsis}
406+
</li>
407+
)
396408
pages.push(
397409
this.renderPagesInInterval(
398410
totalPageNumber! - boundaryCount! + 1,
@@ -409,7 +421,7 @@ class Pagination extends Component<PaginationProps> {
409421
)
410422
)
411423
}
412-
return (<ul css={this.props.styles?.pageIndicatorList}>{pages}</ul>)
424+
return <ul css={this.props.styles?.pageIndicatorList}>{pages}</ul>
413425
}
414426

415427
renderPages(currentPageIndex: number) {
@@ -434,22 +446,22 @@ class Pagination extends Component<PaginationProps> {
434446

435447
if (sliceStart - firstIndex > 1)
436448
visiblePages.unshift(
437-
<span key="first" aria-hidden="true">
438-
&hellip;
439-
</span>
449+
<li style={{ all: 'unset' }} key="first" aria-hidden="true">
450+
{this.props.ellipsis}
451+
</li>
440452
)
441453
if (sliceStart - firstIndex > 0) visiblePages.unshift(firstPage)
442454
if (lastIndex - sliceEnd + 1 > 1)
443455
visiblePages.push(
444-
<span key="last" aria-hidden="true">
445-
&hellip;
446-
</span>
456+
<li style={{ all: 'unset' }} key="last" aria-hidden="true">
457+
{this.props.ellipsis}
458+
</li>
447459
)
448460
if (lastIndex - sliceEnd + 1 > 0) visiblePages.push(lastPage)
449461
}
450462

451463
return (
452-
<View display="inline-block">
464+
<View display="inline-block" as="ul">
453465
{this.transferDisabledPropToChildren(visiblePages)}
454466
</View>
455467
)

0 commit comments

Comments
 (0)