villebro commented on a change in pull request #11979:
URL:
https://github.com/apache/incubator-superset/pull/11979#discussion_r543594946
##########
File path: superset-frontend/src/explore/exploreUtils.js
##########
@@ -248,6 +248,14 @@ export function postForm(url, payload, target = '_blank') {
document.body.removeChild(hiddenForm);
}
+export function columnCount(columnsLength) {
+ let pageSize;
+ if (columnsLength) {
+ pageSize = Math.ceil(Math.max(5, 10000 / columnsLength));
+ }
+ return pageSize || 50;
+}
Review comment:
This function could be named more appropriately, e.g.
`getDataTablePageSize` or similar.
##########
File path: superset-frontend/spec/javascripts/explore/utils_spec.jsx
##########
@@ -200,6 +201,25 @@ describe('exploreUtils', () => {
});
});
+ describe('columnCount', () => {
+ it('divides samples data into pages of 20', () => {
+ const pageSize = columnCount(500);
+ expect(pageSize).toEqual(20);
+ });
+ it('divides samples data into pages of 20', () => {
+ const pageSize = columnCount(0);
+ expect(pageSize).toEqual(50);
+ });
+ it('divides samples data into pages of 20', () => {
+ const pageSize = columnCount(1);
+ expect(pageSize).toEqual(10000);
+ });
+ it('divides samples data into pages of 20', () => {
+ const pageSize = columnCount(1000000);
+ expect(pageSize).toEqual(5);
+ });
+ });
Review comment:
This probably comes down to preference, but I think we could get by with
a single `it` here, just doing multiple
`expect(columnCount(xxx)).toEqual(yyy);` in sequence, as this test is pretty
simple. Just make sure the test description is clear on what's being tested.
But if we do keep separate `it`s here, make sure to update the descriptions
appropriately (all have a reference to 20, which only applies to the first
case).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
