Skip to content

Commit

Permalink
Merge pull request #1724 from hashicorp/pagination-compact-expose-cur…
Browse files Browse the repository at this point in the history 
…rentpagesize

`Pagination` - Fix missing `@currentPageSize` argument in the "Compact" variant with `SizeSelector`
  • Loading branch information
@didoo
didoo committed Oct 19, 2023
2 parents 3ed8e67 + 65ebe6d commit 52c1a62
Show file tree
Hide file tree
Showing 10 changed files with 293 additions and 57 deletions.
5 changes: 5 additions & 0 deletions .changeset/light-clouds-explain.md
View file
@@ -0,0 +1,5 @@
---
"@hashicorp/design-system-components": patch
---

`Pagination` - updated the logic for “Compact” variant to expose `@currentPageSize` and handle controlled/uncontrolled changes
95 changes: 75 additions & 20 deletions packages/components/addon/components/hds/pagination/compact/index.js
View file
Expand Up @@ -4,6 +4,7 @@
*/

import Component from '@glimmer/component';
import { tracked } from '@glimmer/tracking';
import { action } from '@ember/object';
import { assert } from '@ember/debug';
import { inject as service } from '@ember/service';
Expand All @@ -15,6 +16,18 @@ export const DEFAULT_PAGE_SIZES = [10, 30, 50];
export default class HdsPaginationCompactIndexComponent extends Component {
@service router;

// This private variable is used to differentiate between
// "uncontrolled" component (where the state is handled internally) and
// "controlled" component (where the state is handled externally, by the consumer's code).
// In the first case, the variable stores the internal state of the component at any moment,
// and its value is updated internally according to the user's interaction with the component.
// In the second case, the variable stores *only* the initial state of the component (coming from the arguments)
// at rendering time, but from that moment on it's not updated anymore, no matter what interaction the user
// has with the component (the state is controlled externally, eg. via query parameters)
@tracked _currentPageSize = this.args.currentPageSize ?? this.pageSizes[0];
@tracked isControlled;

showLabels = this.args.showLabels ?? true; // if the labels for the "prev/next" controls are visible
showSizeSelector = this.args.showSizeSelector ?? false; // if the "size selector" block is visible

constructor() {
Expand All @@ -31,28 +44,16 @@ export default class HdsPaginationCompactIndexComponent extends Component {
// initialized and updated using the arguments passed to it.

if (queryFunction === undefined) {
this.hasRouting = false;
this.isControlled = false;
} else {
assert(
'@queryFunction for "Hds::Pagination::Numbered" must be a function',
typeof queryFunction === 'function'
);
this.hasRouting = true;
this.isControlled = true;
}
}

/**
* @param showLabels
* @type {boolean}
* @default true
* @description Show the labels for the "prev/next" controls
*/
get showLabels() {
let { showLabels = true } = this.args;

return showLabels;
}

/**
* @param ariaLabel
* @type {string}
Expand All @@ -62,6 +63,35 @@ export default class HdsPaginationCompactIndexComponent extends Component {
return this.args.ariaLabel ?? 'Pagination';
}

// This very specific `get/set` pattern is used to handle the two different use cases of the component
// being "controlled" (when it has routing, meaning it needs to support pagination controls as links/`LinkTo`)
// vs being "uncontrolled" (see comments above for details).
//
// If it has routing (and so it's "controlled"), than the value ("state") of the `currentPageSize` variable
// is *always* determined by the controller via arguments (most of the times, connected to query parameters in the URL).
// For this reason the "get" method always returns the value from the `args`,
// while the "set" method never updates the private internal state (_variable).
//
// If instead it doesn't have routing (and so it's "uncontrolled") than the value ("state") of the `currentPageSize` variables
// is *always* determined by the component's internal logic (and updated according to the user interaction with it).
// For this reason the "get" and "set" methods always read from or write to the private internal state (_variable).

get currentPageSize() {
if (this.isControlled) {
return this.args.currentPageSize;
} else {
return this._currentPageSize;
}
}

set currentPageSize(value) {
if (this.isControlled) {
// noop
} else {
this._currentPageSize = value;
}
}

/**
* @param pageSizes
* @type {array of numbers}
Expand All @@ -83,9 +113,9 @@ export default class HdsPaginationCompactIndexComponent extends Component {
return this.router.currentRoute?.queryParams || {};
}

buildQueryParamsObject(page) {
if (this.hasRouting) {
return this.args.queryFunction(page, this.currentPage);
buildQueryParamsObject(page, pageSize) {
if (this.isControlled) {
return this.args.queryFunction(page, pageSize);
} else {
return {};
}
Expand All @@ -100,9 +130,15 @@ export default class HdsPaginationCompactIndexComponent extends Component {
};

// the "query" is dynamic and needs to be calculated
if (this.hasRouting) {
routing.queryPrev = this.buildQueryParamsObject('prev');
routing.queryNext = this.buildQueryParamsObject('next');
if (this.isControlled) {
routing.queryPrev = this.buildQueryParamsObject(
'prev',
this.currentPageSize
);
routing.queryNext = this.buildQueryParamsObject(
'next',
this.currentPageSize
);
} else {
routing.queryPrev = undefined;
routing.queryNext = undefined;
Expand All @@ -121,4 +157,23 @@ export default class HdsPaginationCompactIndexComponent extends Component {
onPageChange(newPage);
}
}

@action
onPageSizeChange(newPageSize) {
let { onPageSizeChange } = this.args;

// we need to manually update the query parameters in the route (it's not a link!)
if (this.isControlled) {
// we pass `null` as value for the `page` argument, so consumers can handle this condition accordingly (probably will just change the side of the data/array slice)
const queryParams = this.buildQueryParamsObject(null, newPageSize);
this.router.transitionTo({ queryParams });
} else {
this.currentPageSize = newPageSize;
}

// invoke the callback function
if (typeof onPageSizeChange === 'function') {
onPageSizeChange(newPageSize);
}
}
}
26 changes: 13 additions & 13 deletions packages/components/addon/components/hds/pagination/numbered/index.js
View file
Expand Up @@ -85,6 +85,7 @@ export default class HdsPaginationNumberedIndexComponent extends Component {
// has with the component (the state is controlled externally, eg. via query parameters)
@tracked _currentPage = this.args.currentPage ?? 1;
@tracked _currentPageSize = this.args.currentPageSize ?? this.pageSizes[0];
@tracked isControlled;

showInfo = this.args.showInfo ?? true; // if the "info" block is visible
showLabels = this.args.showLabels ?? false; // if the labels for the "prev/next" controls are visible
Expand All @@ -107,7 +108,7 @@ export default class HdsPaginationNumberedIndexComponent extends Component {
// initialized and updated using the arguments passed to it.

if (queryFunction === undefined) {
this.hasRouting = false;
this.isControlled = false;
} else {
assert(
'@queryFunction for "Hds::Pagination::Numbered" must be a function',
Expand All @@ -118,7 +119,7 @@ export default class HdsPaginationNumberedIndexComponent extends Component {
typeof this.args.currentPageSize === 'number' &&
typeof this.args.currentPage === 'number'
);
this.hasRouting = true;
this.isControlled = true;
}

assert(
Expand All @@ -137,7 +138,7 @@ export default class HdsPaginationNumberedIndexComponent extends Component {
}

// This very specific `get/set` pattern is used to handle the two different use cases of the component
// being "controlled" (when it has routing, meaning it needs to support links as controls)
// being "controlled" (when it has routing, meaning it needs to support pagination controls as links/`LinkTo`)
// vs being "uncontrolled" (see comments above for details).
//
// If it has routing (and so it's "controlled"), than the value ("state") of the `currentPage/currentPageSize` variables
Expand All @@ -150,31 +151,31 @@ export default class HdsPaginationNumberedIndexComponent extends Component {
// For this reason the "get" and "set" methods always read from or write to the private internal state (_variable).

get currentPage() {
if (this.hasRouting) {
if (this.isControlled) {
return this.args.currentPage;
} else {
return this._currentPage;
}
}

set currentPage(value) {
if (this.hasRouting) {
if (this.isControlled) {
// noop
} else {
this._currentPage = value;
}
}

get currentPageSize() {
if (this.hasRouting) {
if (this.isControlled) {
return this.args.currentPageSize;
} else {
return this._currentPageSize;
}
}

set currentPageSize(value) {
if (this.hasRouting) {
if (this.isControlled) {
// noop
} else {
this._currentPageSize = value;
Expand Down Expand Up @@ -242,7 +243,7 @@ export default class HdsPaginationNumberedIndexComponent extends Component {
}

buildQueryParamsObject(page, pageSize) {
if (this.hasRouting) {
if (this.isControlled) {
return this.args.queryFunction(page, pageSize);
} else {
return {};
Expand All @@ -258,7 +259,7 @@ export default class HdsPaginationNumberedIndexComponent extends Component {
};

// the "query" is dynamic and needs to be calculated
if (this.hasRouting) {
if (this.isControlled) {
routing.queryPrev = this.buildQueryParamsObject(
this.currentPage - 1,
this.currentPageSize
Expand Down Expand Up @@ -323,10 +324,9 @@ export default class HdsPaginationNumberedIndexComponent extends Component {
let { onPageSizeChange } = this.args;

// we need to manually update the query parameters in the route (it's not a link!)
// notice: we agreed to reset the pagination to the first element (any alternative would result in an unpredictable UX)
if (this.hasRouting) {
let queryParams = Object.assign({}, this.routeQueryParams);
queryParams = this.buildQueryParamsObject(1, newPageSize);
if (this.isControlled) {
// notice: we agreed to reset the pagination to the first element (any alternative would result in an unpredictable UX)
const queryParams = this.buildQueryParamsObject(1, newPageSize);
this.router.transitionTo({ queryParams });
} else {
this.currentPage = 1;
Expand Down
103 changes: 101 additions & 2 deletions packages/components/tests/acceptance/components/hds/pagination-test.js
View file
Expand Up @@ -72,27 +72,126 @@ module('Acceptance | Component | hds/pagination', function (hooks) {
await visit('/components/pagination');

assert.strictEqual(currentURL(), '/components/pagination');

assert
.dom(
'#demo4-compact-with-routing .hds-table .hds-table__tbody .hds-table__tr'
)
.exists({ count: 5 });
assert
.dom(
'#demo4-compact-with-routing .hds-table .hds-table__tbody .hds-table__tr .hds-table__td'
)
.hasText('1');
assert
.dom(
'#demo4-compact-with-routing .hds-table .hds-table__tbody .hds-table__tr:nth-child(5) .hds-table__td'
)
.hasText('5');

// CLICK "NEXT"
// ------------

await click(
'#demo4-compact-with-routing .hds-pagination .hds-pagination-nav__arrow--direction-next'
);

assert.strictEqual(
currentURL(),
'/components/pagination?demoExtraParam=hello&nextCursor_demo4=bmV4dF9fNg%3D%3D'
);
assert
.dom(
'#demo4-compact-with-routing .hds-table .hds-table__tbody .hds-table__tr'
)
.exists({ count: 5 });
assert
.dom(
'#demo4-compact-with-routing .hds-table .hds-table__tbody .hds-table__tr .hds-table__td'
)
.hasText('6');
assert
.dom(
'#demo4-compact-with-routing .hds-table .hds-table__tbody .hds-table__tr:nth-child(5) .hds-table__td'
)
.hasText('10');

// CHANGE PAGE SIZE
// ----------------

await select(
'#demo4-compact-with-routing .hds-pagination .hds-pagination-size-selector select',
'10'
);
assert
.dom(
'#demo4-compact-with-routing .hds-table .hds-table__tbody .hds-table__tr'
)
.exists({ count: 10 });
assert.strictEqual(
currentURL(),
'/components/pagination?currentPageSize_demo4=10&demoExtraParam=hello&nextCursor_demo4=bmV4dF9fNg%3D%3D'
);
assert
.dom(
'#demo4-compact-with-routing .hds-table .hds-table__tbody .hds-table__tr .hds-table__td'
)
.hasText('6');
assert
.dom(
'#demo4-compact-with-routing .hds-table .hds-table__tbody .hds-table__tr:nth-child(10) .hds-table__td'
)
.hasText('15');

// CLICK "NEXT" + "PREV"
// ------------

await click(
'#demo4-compact-with-routing .hds-pagination .hds-pagination-nav__arrow--direction-next'
);
await click(
'#demo4-compact-with-routing .hds-pagination .hds-pagination-nav__arrow--direction-prev'
);
assert.strictEqual(
currentURL(),
'/components/pagination?currentPageSize_demo4=10&demoExtraParam=hello&prevCursor_demo4=cHJldl9fMTY%3D'
);
assert
.dom(
'#demo4-compact-with-routing .hds-table .hds-table__tbody .hds-table__tr .hds-table__td'
)
.hasText('6');
assert
.dom(
'#demo4-compact-with-routing .hds-table .hds-table__tbody .hds-table__tr:nth-child(10) .hds-table__td'
)
.hasText('15');

// CLICK "PREV"
// ------------
// this is a special test to make sure that when the prev cursor is less than the page size the demo code still works (there was a bug before)

await click(
'#demo4-compact-with-routing .hds-pagination .hds-pagination-nav__arrow--direction-prev'
);
assert.strictEqual(
currentURL(),
'/components/pagination?demoExtraParam=hello&prevCursor_demo4=cHJldl9fNg%3D%3D'
'/components/pagination?currentPageSize_demo4=10&demoExtraParam=hello&prevCursor_demo4=cHJldl9fNg%3D%3D'
);
assert
.dom(
'#demo4-compact-with-routing .hds-table .hds-table__tbody .hds-table__tr'
)
// notice: even if the "page size" is 10, we see only 5 records because we are counting "10 records before record #6" and so only 5 records exist
.exists({ count: 5 });
assert
.dom(
'#demo4-compact-with-routing .hds-table .hds-table__tbody .hds-table__tr .hds-table__td'
)
.hasText('1');
assert
.dom(
'#demo4-compact-with-routing .hds-table .hds-table__tbody .hds-table__tr:nth-child(5) .hds-table__td'
)
.hasText('5');
});
});

2 comments on commit 52c1a62

@vercel
Copy link

@vercel vercel bot commented on 52c1a62 Oct 19, 2023

Choose a reason for hiding this comment

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

Successfully deployed to the following URLs:

hds-showcase – ./packages/components

hds-showcase.vercel.app
hds-components-hashicorp.vercel.app
hds-showcase-hashicorp.vercel.app
hds-showcase-git-main-hashicorp.vercel.app

@vercel
Copy link

@vercel vercel bot commented on 52c1a62 Oct 19, 2023

Choose a reason for hiding this comment

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

Successfully deployed to the following URLs:

hds-website – ./website

design-system-components-hashicorp.vercel.app
hds-website-git-main-hashicorp.vercel.app
hds-website.vercel.app
flight-hashicorp.vercel.app
helios.hashicorp.design
hds-website-hashicorp.vercel.app

Please sign in to comment.