Skip to content
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.

Sign up for GitHub

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

Hide paginationBar in EuiBasicTable when there is no data #2598

Merged
merged 13 commits into from
Dec 9, 2019
+77 −21
Merged

Hide paginationBar in EuiBasicTable when there is no data #2598

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
dbc0655
hiding pagination when needed
andreadelrio Dec 5, 2019
a005546
Merge remote-tracking branch 'upstream/master' into rowsPerPage
andreadelrio Dec 5, 2019
b55ef3f
hiding pagination when there are not enough rows
andreadelrio Dec 6, 2019
243a566
cl
andreadelrio Dec 6, 2019
2bbd44d
show EuiPagination when there is 1 page
andreadelrio Dec 6, 2019
4e6fdd8
Merge remote-tracking branch 'upstream/master' into rowsPerPage
andreadelrio Dec 6, 2019
664bf2b
update cl
andreadelrio Dec 6, 2019
d63690f
extra line
andreadelrio Dec 6, 2019
d98a56f
cleanup
andreadelrio Dec 9, 2019
0bc2686
updated cl
andreadelrio Dec 9, 2019
60286e2
fix ts error
andreadelrio Dec 9, 2019
3d195a2
updated snaps
andreadelrio Dec 9, 2019
3616b1d
fix cl conflict
andreadelrio Dec 9, 2019
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
**Bug fixes**

- Fixed UX/focus bug in `EuiDataGrid` when using keyboard shortcuts to paginate ([#2602](https://github.com/elastic/eui/pull/2602))
- Hide `paginationBar` in `EuiBasicTable` when there is no data ([#2598](https://github.com/elastic/eui/pull/#2598))
- Display `EuiPagination` in `EuiBasicTable` when there is only 1 page ([#2598](https://github.com/elastic/eui/pull/#2598))
Copy link
Contributor

Choose a reason for hiding this comment

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

These entries can be rolled into one because they both are affecting EuiBasicTable pagination options


## [`17.0.0`](https://github.com/elastic/eui/tree/v17.0.0)

Expand Down
6 changes: 3 additions & 3 deletions src/components/basic_table/basic_table.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ export class EuiBasicTable extends Component {
);

const table = this.renderTable();
const paginationBar = this.renderPaginationBar();
const paginationBar = this.renderPaginationBar(items);
Copy link
Contributor

Choose a reason for hiding this comment

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

A better thing to pass here would be the length of items so that there's an understanding of what the renderPaginationBar() method is doing with that parameter which is just checking that items to do exist.

Suggested change
const paginationBar = this.renderPaginationBar(items);
const paginationBar = this.renderPaginationBar(items.length);


return (
<div className={classes} {...rest}>
Expand Down Expand Up @@ -1049,9 +1049,9 @@ export class EuiBasicTable extends Component {
return profile.align;
}

renderPaginationBar() {
renderPaginationBar(items) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Then here, you can change the name to something like

Suggested change
renderPaginationBar(items) {
renderPaginationBar(numItems) {

const { error, pagination, onChange } = this.props;
if (!error && pagination) {
if (!error && pagination && items.length > 0) {
if (!onChange) {
throw new Error(`The Basic Table is configured with pagination but [onChange] is
not configured. This callback must be implemented to handle pagination changes`);
Expand Down
58 changes: 57 additions & 1 deletion src/components/pagination/__snapshots__/pagination.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,59 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`EuiPagination is rendered 1`] = `<span />`;
exports[`EuiPagination is rendered 1`] = `
<div
aria-label="aria-label"
class="euiPagination testClass1 testClass2"
data-test-subj="test subject string"
role="group"
>
<button
aria-label="Previous page"
class="euiButtonIcon euiButtonIcon--text"
data-test-subj="pagination-button-previous"
type="button"
>
<svg
aria-hidden="true"
class="euiIcon euiIcon--medium euiIcon-isLoading euiButtonIcon__icon"
focusable="false"
height="16"
viewBox="0 0 16 16"
width="16"
xmlns="http://www.w3.org/2000/svg"
/>
</button>
<button
aria-label="Page 1 of 1"
class="euiButtonEmpty euiButtonEmpty--text euiButtonEmpty--xSmall euiPaginationButton euiPaginationButton--hideOnMobile"
data-test-subj="pagination-button-0"
type="button"
>
<span
class="euiButtonEmpty__content"
>
<span
class="euiButtonEmpty__text"
>
1
</span>
</span>
</button>
<button
aria-label="Next page"
class="euiButtonIcon euiButtonIcon--text"
data-test-subj="pagination-button-next"
type="button"
>
<svg
aria-hidden="true"
class="euiIcon euiIcon--medium euiIcon-isLoading euiButtonIcon__icon"
focusable="false"
height="16"
viewBox="0 0 16 16"
width="16"
xmlns="http://www.w3.org/2000/svg"
/>
</button>
</div>
`;
2 changes: 1 addition & 1 deletion src/components/pagination/pagination.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ export const EuiPagination: FunctionComponent<Props> = ({
</EuiI18n>
);

if (pages.length > 1) {
if (pages.length > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

pages.length will never be less than 0 so this statement will always return true, which means that it's an unnecessary check the component should always return the full pagination and never an empty span.

const selectablePages = pages;
if (compressed) {
return (
Expand Down