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.

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

bugfix: first multiselection has no _lastSelectedIndex #788

Merged
merged 1 commit into from
Nov 22, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 13 additions & 2 deletions addon-test-support/pages/ember-table.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,24 @@ export default PageObject.extend({
},

/**
* Selects a range of rows in the body
* Selects a range of rows in the body with simple click first
*
* @param {number} beginIndex
* @param {number} endIndex
*/
async selectRange(beginIndex, endIndex) {
async selectRangeFromClick(beginIndex, endIndex) {
await this.body.rows.objectAt(beginIndex).click();
await this.body.rows.objectAt(endIndex).clickWith({ shiftKey: true });
},

/**
* Selects a range of rows in the body with shift+click first
*
* @param {number} beginIndex
* @param {number} endIndex
*/
async selectRangeFromShiftClick(beginIndex, endIndex) {
await this.body.rows.objectAt(beginIndex).clickWith({ shiftKey: true });
await this.body.rows.objectAt(endIndex).clickWith({ shiftKey: true });
},
});
6 changes: 4 additions & 2 deletions addon/-private/collapse-tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,10 @@ export const TableRowMeta = EmberObject.extend({
// Use a set to avoid item duplication
let { _lastSelectedIndex } = tree;

let minIndex = Math.min(_lastSelectedIndex, rowIndex);
let maxIndex = Math.max(_lastSelectedIndex, rowIndex);
let isFirstIndexDefined = typeof _lastSelectedIndex === 'number';

let minIndex = isFirstIndexDefined ? Math.min(_lastSelectedIndex, rowIndex) : rowIndex;
let maxIndex = isFirstIndexDefined ? Math.max(_lastSelectedIndex, rowIndex) : rowIndex;

for (let i = minIndex; i <= maxIndex; i++) {
selection.add(tree.objectAt(i));
Expand Down
24 changes: 17 additions & 7 deletions tests/integration/components/selection-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,12 +115,22 @@ module('Integration | selection', () => {
assert.ok(table.validateSelected(), 'the rows are toggled back off');
});

test('Can select a range with shift', async function(assert) {
test('Can select a range with shift from click', async function(assert) {
await generateTable(this);

assert.ok(table.validateSelected(), 'rows are not selected');

await table.selectRange(0, 2);
await table.selectRangeFromClick(0, 2);

assert.ok(table.validateSelected(0, 1, 2), 'rows are selected');
});

test('Can select a range with shift from shift-click', async function(assert) {
await generateTable(this);

assert.ok(table.validateSelected(), 'rows are not selected');

await table.selectRangeFromShiftClick(0, 2);

assert.ok(table.validateSelected(0, 1, 2), 'rows are selected');
});
Expand All @@ -130,7 +140,7 @@ module('Integration | selection', () => {

assert.ok(table.validateSelected(), 'rows are not selected');

await table.selectRange(3, 5);
await table.selectRangeFromClick(3, 5);

assert.ok(table.validateSelected(3, 4, 5), 'rows are selected');
});
Expand All @@ -144,7 +154,7 @@ module('Integration | selection', () => {

assert.ok(table.validateSelected(4), 'middle row selected');

await table.selectRange(3, 5);
await table.selectRangeFromClick(3, 5);

assert.ok(table.validateSelected(3, 4, 5), 'all rows are selected');
});
Expand Down Expand Up @@ -288,7 +298,7 @@ module('Integration | selection', () => {

assert.ok(table.validateSelected(), 'no rows are not selected');

await table.selectRange(0, 3);
await table.selectRangeFromClick(0, 3);

assert.ok(table.validateSelected(3), 'last row only is selected');
});
Expand Down Expand Up @@ -473,7 +483,7 @@ module('Integration | selection', () => {

assert.ok(table.validateSelected(), 'rows are not selected');

await table.selectRange(1, 3);
await table.selectRangeFromClick(1, 3);

assert.ok(table.validateSelected(0, 1, 2, 3), 'row and its children are selected');
});
Expand All @@ -487,7 +497,7 @@ module('Integration | selection', () => {

assert.ok(table.validateSelected(), 'rows are not selected');

await table.selectRange(1, 3);
await table.selectRangeFromClick(1, 3);

assert.ok(table.validateSelected(1, 2, 3), 'only children are selected');
});
Expand Down