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

Widget resize tests #3620

Merged
merged 9 commits into from
Mar 28, 2019
Merged

Widget resize tests #3620

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
302ae48
Widget resize tests
ranbena Mar 23, 2019
957e99f
Added 2 new auto height and resize tests
ranbena Mar 25, 2019
7613912
Update client/cypress/integration/dashboard/dashboard_spec.js
gabrieldutra Mar 26, 2019
5ee6087
Update client/cypress/integration/dashboard/dashboard_spec.js
gabrieldutra Mar 26, 2019
c2bd514
Update client/cypress/integration/dashboard/dashboard_spec.js
gabrieldutra Mar 26, 2019
cb84016
Update client/cypress/integration/dashboard/dashboard_spec.js
gabrieldutra Mar 26, 2019
97670c9
Update client/cypress/integration/dashboard/dashboard_spec.js
gabrieldutra Mar 26, 2019
2c747a2
Update client/cypress/integration/dashboard/dashboard_spec.js
gabrieldutra Mar 26, 2019
0550fa3
Swapped left and top
ranbena Mar 26, 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: 1 addition & 1 deletion client/app/components/dashboards/widget.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
</div>

<div class="body-row clearfix tile__bottom-control">
<a class="small hidden-print" ng-click="$ctrl.refresh()" ng-if="!$ctrl.public">
<a class="small hidden-print" ng-click="$ctrl.refresh()" ng-if="!$ctrl.public" data-test="RefreshIndicator">
<i ng-class='{"zmdi-hc-spin": $ctrl.widget.loading}' class="zmdi zmdi-refresh"></i>
<span am-time-ago="$ctrl.widget.getQueryResult().getUpdatedAt()" ng-if="!$ctrl.widget.loading"></span>
<rd-timer timestamp="$ctrl.widget.refreshStartedAt" ng-if="$ctrl.widget.loading"></rd-timer>
Expand Down
2 changes: 1 addition & 1 deletion client/app/components/parameters.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
ui-sortable="{ 'ui-floating': true, 'disabled': !editable }"
ng-model="parameters"
>
<div class="form-group m-r-10" ng-repeat="param in parameters">
<div class="form-group m-r-10" ng-repeat="param in parameters" data-test="ParameterName{{ param.name }}">
<label class="parameter-label">{{param.title}}</label>
<button class="btn btn-default btn-xs" ng-if="editable" ng-click="showParameterSettings(param, $index)">
<i class="zmdi zmdi-settings"></i>
Expand Down
4 changes: 2 additions & 2 deletions client/app/pages/dashboards/dashboard.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ <h3>
ng-class="{'preview-mode': !$ctrl.layoutEditing, 'editing-mode': $ctrl.layoutEditing}">
<div class="dashboard-widget-wrapper"
ng-repeat="widget in $ctrl.dashboard.widgets track by widget.id"
gridstack-item="widget.options.position" gridstack-item-id="{{ widget.id }}">
<div class="grid-stack-item-content" data-test="WidgetId{{ widget.id }}">
gridstack-item="widget.options.position" gridstack-item-id="{{ widget.id }}" data-test="WidgetId{{ widget.id }}">
<div class="grid-stack-item-content">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to parent node in order to locate the handle (which is a sibling)

https://github.com/getredash/redash/blob/689e1a63bff22e5e7cdc354890e5c576cd307ea1/cypress/integration/dashboard/dashboard_spec.js#L112-L116

<dashboard-widget widget="widget" dashboard="$ctrl.dashboard" on-delete="$ctrl.removeWidget(widget.id)"></dashboard-widget>
</div>
</div>
Expand Down
207 changes: 200 additions & 7 deletions client/cypress/integration/dashboard/dashboard_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
const DRAG_PLACEHOLDER_SELECTOR = '.grid-stack-placeholder';
const RESIZE_HANDLE_SELECTOR = '.ui-resizable-se';


function createNewDashboardByAPI(name) {
return cy.request('POST', 'api/dashboards', { name })
Expand Down Expand Up @@ -78,7 +80,7 @@ function addWidgetByAPI(dashId, queryData = {}) {
});
}

function dragBy(wrapper, offsetTop = 0, offsetLeft = 0) {
function dragBy(wrapper, offsetLeft = 0, offsetTop = 0) {
let start;
let end;
return wrapper
Expand All @@ -101,6 +103,34 @@ function dragBy(wrapper, offsetTop = 0, offsetLeft = 0) {
}));
}

function resizeBy(wrapper, offsetLeft = 0, offsetTop = 0) {
let start;
let end;
let from;
const getSize = $el => ({ height: $el.height(), width: $el.width() });

return wrapper
.then(($el) => {
start = getSize($el);
})
.within(() => cy.get(RESIZE_HANDLE_SELECTOR))
.then(($handle) => {
from = $handle.show().offset(); // turn on handle and get it's position
return wrapper
.trigger('mouseover', { force: true })
.trigger('mousedown', { pageX: from.left, pageY: from.top, force: true, which: 1 })
.trigger('mousemove', { pageX: from.left + offsetLeft, pageY: from.top + offsetTop, force: true, which: 1 });
})
.then(() => {
end = getSize(Cypress.$(DRAG_PLACEHOLDER_SELECTOR)); // see comment in dragBy ^^
return wrapper.trigger('mouseup', { force: true });
})
.then(() => ({
height: end.height - start.height,
width: end.width - start.width,
}));
}

describe('Dashboard', () => {
beforeEach(() => {
cy.login();
Expand Down Expand Up @@ -268,24 +298,24 @@ describe('Dashboard', () => {

describe('Draggable', () => {
describe('Grid snap', () => {
beforeEach(function () {
beforeEach(() => {
editDashboard();
});

it('stays put when dragged under snap threshold', () => {
dragBy(cy.get('@textboxEl'), 0, 90).then((delta) => {
dragBy(cy.get('@textboxEl'), 90).then((delta) => {
expect(delta.left).to.eq(0);
});
});

it('moves one column when dragged over snap threshold', () => {
dragBy(cy.get('@textboxEl'), 0, 110).then((delta) => {
dragBy(cy.get('@textboxEl'), 110).then((delta) => {
expect(delta.left).to.eq(200);
});
});

it('moves two columns when dragged over snap threshold', () => {
dragBy(cy.get('@textboxEl'), 0, 330).then((delta) => {
dragBy(cy.get('@textboxEl'), 330).then((delta) => {
expect(delta.left).to.eq(400);
});
});
Expand All @@ -298,7 +328,7 @@ describe('Dashboard', () => {
.then(($el) => {
start = $el.offset();
editDashboard();
return dragBy(cy.get('@textboxEl'), 0, 200);
return dragBy(cy.get('@textboxEl'), 200);
})
// cancel
.then(() => {
Expand All @@ -320,7 +350,7 @@ describe('Dashboard', () => {
.then(($el) => {
start = $el.offset();
editDashboard();
return dragBy(cy.get('@textboxEl'), 0, 200);
return dragBy(cy.get('@textboxEl'), 200);
})
// apply
.then(() => {
Expand All @@ -333,6 +363,104 @@ describe('Dashboard', () => {
});
});
});

describe('Resizeable', () => {
describe('Column snap', () => {
beforeEach(() => {
editDashboard();
});

it('stays put when dragged under snap threshold', () => {
resizeBy(cy.get('@textboxEl'), 90).then((delta) => {
expect(delta.width).to.eq(0);
});
});

it('moves one column when dragged over snap threshold', () => {
resizeBy(cy.get('@textboxEl'), 110).then((delta) => {
expect(delta.width).to.eq(200);
});
});

it('moves two columns when dragged over snap threshold', () => {
resizeBy(cy.get('@textboxEl'), 400).then((delta) => {
expect(delta.width).to.eq(400);
});
});
});

describe('Row snap', () => {
beforeEach(() => {
editDashboard();
});

it('stays put when dragged under snap threshold', () => {
resizeBy(cy.get('@textboxEl'), 0, 10).then((delta) => {
expect(delta.height).to.eq(0);
});
});

it('moves one row when dragged over snap threshold', () => {
resizeBy(cy.get('@textboxEl'), 0, 30).then((delta) => {
expect(delta.height).to.eq(50);
});
});

it('shrinks to minimum', () => {
cy.get('@textboxEl')
.then(($el) => {
resizeBy(cy.get('@textboxEl'), -$el.width(), -$el.height()); // resize to 0,0
return cy.get('@textboxEl');
})
.then(($el) => {
expect($el.width()).to.eq(200);
expect($el.height()).to.eq(35);
});
});
});

it('discards resize on cancel', () => {
let start;
cy.get('@textboxEl')
// save initial position, resize textbox 1 col
.then(($el) => {
start = $el.height();
editDashboard();
return resizeBy(cy.get('@textboxEl'), 0, 200);
})
// cancel
.then(() => {
cy.get('.dashboard-header').within(() => {
cy.contains('button', 'Cancel').click();
});
return cy.get('@textboxEl');
})
// verify returned to original size
.then(($el) => {
expect($el.height()).to.eq(start);
});
});

it('saves resize on apply', () => {
let start;
cy.get('@textboxEl')
// save initial position, resize textbox 1 col
.then(($el) => {
start = $el.height();
editDashboard();
return resizeBy(cy.get('@textboxEl'), 0, 200);
})
// apply
.then(() => {
cy.contains('button', 'Apply Changes').click().should('not.exist');
return cy.get('@textboxEl');
})
// verify size change persists
.then(($el) => {
expect($el.height()).to.not.eq(start);
});
});
});
});

describe('Widget', () => {
Expand Down Expand Up @@ -395,6 +523,71 @@ describe('Dashboard', () => {
.should('eq', 335);
});
});

describe('Height behavior on refresh', () => {
const paramName = 'count';
const queryData = {
query: `select s.a FROM generate_series(1,{{ ${paramName} }}) AS s(a)`,
};

beforeEach(function () {
addWidgetByAPI(this.dashboardId, queryData).then((elTestId) => {
cy.visit(this.dashboardUrl);
cy.getByTestId(elTestId).as('widget').within(() => {
cy.getByTestId('RefreshIndicator').as('refreshButton');
});
cy.getByTestId(`ParameterName${paramName}`).within(() => {
cy.get('input').as('paramInput');
});
});
});

it('grows when dynamically adding table rows', () => {
// listen to results
cy.server();
cy.route('GET', 'api/query_results/*').as('FreshResults');

// start with 1 table row
cy.get('@paramInput').clear().type('1');
cy.get('@refreshButton').click();
cy.wait('@FreshResults', { timeout: 10000 });
cy.get('@widget').invoke('height').should('eq', 285);

// add 4 table rows
cy.get('@paramInput').clear().type('5');
cy.get('@refreshButton').click();
cy.wait('@FreshResults', { timeout: 10000 });

// expect to height to grow by 1 grid grow
cy.get('@widget').invoke('height').should('eq', 435);
});

it('revokes auto height after manual height adjustment', () => {
// listen to results
cy.server();
cy.route('GET', 'api/query_results/*').as('FreshResults');

editDashboard();

// start with 1 table row
cy.get('@paramInput').clear().type('1');
cy.get('@refreshButton').click();
cy.wait('@FreshResults');
cy.get('@widget').invoke('height').should('eq', 285);

// resize height by 1 grid row
resizeBy(cy.get('@widget'), 0, 5);
cy.get('@widget').invoke('height').should('eq', 335);

// add 4 table rows
cy.get('@paramInput').clear().type('5');
cy.get('@refreshButton').click();
cy.wait('@FreshResults');

// expect height to stay unchanged (would have been 435)
Copy link
Member

Choose a reason for hiding this comment

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

Just to show a different way to assert the same thing:

expect(() => {
  // add 4 table rows
  cy.get('@paramInput').clear().type('5');
  cy.get('@refreshButton').click();
  cy.wait('@FreshResults');
}).not.to.change(this.widget, 'height');

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oooh that's nice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But I don't think this specifically would work cause

  1. height is a method, not a property.
  2. this.widget is a cy object, not jquery.

Right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ideally the dragBy and resizeBy functions would not return a delta value but instead:

it('moves one column', () => {
  const getLeft = () => $el.offset().left;
  const drag = () => dragBy($el, 200);

  expect(drag).to.increase(getLeft).by(200);
});

Then resizeBy would simply be a dragBy of the element's handle. So nice.

BUT, disconnecting the drag from the delta calculation is currently problematic since the dragged element animates to its new position on mouseup (compensated by calculating the drag placeholder position instead).
Any workaround suggestion? (wait for animation end, disable animation for test, ...)

Copy link
Member

Choose a reason for hiding this comment

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

Really cool, once I get to my laptop I'll run a few tests with it. For the questions:

But I don't think this specifically would work cause

It worked for success, need to try on failure haha. The assertion is essentially the same, they just run it before the function and then guarantee it's the same after.

height is a method, not a property

I hope not to have seen the above wrongly, but this is not supposed to be a problem

this.widget is a cy object, not jquery.

Need to confirm, but I think it is a jQuery object, when I saw it in docs it seemed the same as getting the result of cy.get('@widget')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you can make it work that would be awesome 👍

Copy link
Member

Choose a reason for hiding this comment

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

I could not 😅. That works well with javascript objects, not with elements (I could use a helper object to represent the element, but I don't think it is worth it for my example).

Yours is much better, it would make tests a lot more readable.

Any workaround suggestion?

Can we use the placeholder position for the getLeft method? If not, NP, we can move on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we use the placeholder position for the getLeft method? If not, NP, we can move on.

It doesn't get rendered in the dom until mousedown (or mousemove, I didn't check) so it's not ideal.

cy.get('@widget').invoke('height').should('eq', 335);
});
});
});
});
});