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

[FEATURE] multiSelectRequiresKeyboard option for toggling row selec… #160

Merged
merged 1 commit into from
Sep 7, 2016
Merged

[FEATURE] multiSelectRequiresKeyboard option for toggling row selec… #160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 15 additions & 1 deletion addon/components/lt-body.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,19 @@ export default Component.extend({
*/
multiSelect: false,

/**
* When multiSelect is true, this property determines whether or not `ctrl`
* (or `cmd`) is required to select additional rows, one by one. When false,
* simply clicking on subsequent rows will select or deselect them.
*
* `shift` to select many consecutive rows is unaffected by this property.
*
* @property multiSelectRequiresKeyboard
* @type {Boolean}
* @default true
*/
multiSelectRequiresKeyboard: true,

/**
* Hide scrollbar when not scrolling
*
Expand Down Expand Up @@ -206,6 +219,7 @@ export default Component.extend({
onRowClick(row, e) {
let rows = this.get('table.rows');
let multiSelect = this.get('multiSelect');
let multiSelectRequiresKeyboard = this.get('multiSelectRequiresKeyboard');
let canSelect = this.get('canSelect');
let isSelected = row.get('selected');
let currIndex = rows.indexOf(row);
Expand All @@ -218,7 +232,7 @@ export default Component.extend({
if (e.shiftKey && multiSelect) {
rows.slice(Math.min(currIndex, prevIndex), Math.max(currIndex, prevIndex) + 1).forEach(r => r.set('selected', !isSelected));
this._prevSelectedIndex = currIndex;
} else if ((e.ctrlKey || e.metaKey) && multiSelect) {
} else if ((!multiSelectRequiresKeyboard || (e.ctrlKey || e.metaKey)) && multiSelect) {
row.toggleProperty('selected');
} else {
this.get('table.selectedRows').setEach('selected', false);
Expand Down
36 changes: 30 additions & 6 deletions tests/integration/components/lt-body-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ test('it renders', function(assert) {
assert.equal(this.$().text().trim(), '');
});

test('row selection', function(assert) {
test('row selection - enable or disable', function(assert) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

rather than have three tests called 'row selection' I took the liberty of adding additional detail to these test names to differentiate them

this.set('table', new Table(Columns, createUsers(1)));
this.set('canSelect', false);

Expand All @@ -45,7 +45,7 @@ test('row selection', function(assert) {
assert.ok(row.hasClass('is-selected'));
});

test('row selection', function(assert) {
test('row selection - ctrl-click to modify selection', function(assert) {
this.set('table', new Table(Columns, createUsers(5)));

this.render(hbs `{{lt-body table=table sharedOptions=sharedOptions canSelect=true multiSelect=true}}`);
Expand All @@ -57,16 +57,40 @@ test('row selection', function(assert) {
assert.equal(this.$('tbody > tr').length, 5);

firstRow.click();
assert.equal(this.$('tr.is-selected').length, 1);
assert.equal(this.$('tr.is-selected').length, 1, 'clicking a row selects it');

lastRow.trigger(createClickEvent({shiftKey: true}));
assert.equal(this.$('tr.is-selected').length, 5);
assert.equal(this.$('tr.is-selected').length, 5, 'shift-clicking another row selects it and all rows between');

middleRow.trigger(createClickEvent({ctrlKey: true}));
assert.equal(this.$('tr.is-selected').length, 4);
assert.equal(this.$('tr.is-selected').length, 4, 'ctrl-clicking a selected row deselects it');

firstRow.click();
assert.equal(this.$('tr.is-selected').length, 0);
assert.equal(this.$('tr.is-selected').length, 0, 'clicking a selected row deselects all rows');
});

test('row selection - click to modify selection', function(assert) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this new test is very much based on the standard mode test before it, exercising and documenting the differences in behavior for (nearly) identical user input

this.set('table', new Table(Columns, createUsers(5)));

this.render(hbs `{{lt-body table=table sharedOptions=sharedOptions canSelect=true multiSelect=true multiSelectRequiresKeyboard=false}}`);

let firstRow = this.$('tr:first');
let middleRow = this.$('tr:nth-of-type(3)');
let lastRow = this.$('tr:last');

assert.equal(this.$('tbody > tr').length, 5);

firstRow.click();
assert.equal(this.$('tr.is-selected').length, 1, 'clicking a row selects it');

lastRow.trigger(createClickEvent({shiftKey: true}));
assert.equal(this.$('tr.is-selected').length, 5, 'shift-clicking another row selects it and all rows between');

middleRow.click();
assert.equal(this.$('tr.is-selected').length, 4, 'clicking a selected row deselects it without affecting other selected rows');

middleRow.click();
assert.equal(this.$('tr.is-selected').length, 5, 'clicking a deselected row selects it without affecting other selected rows');
});

test('row expansion', function(assert) {
Expand Down