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

feat(data-table): indeterminate state in 'selectAll' checkbox (closes #571) #573

Merged
merged 2 commits into from
May 8, 2017

Conversation

emoralesb05
Copy link
Contributor

when at least one row is selected and not every row is selected, the checkbox for select/deselect all should be in indeterminate state.

#571

What's included?

  • Show indeterminate state when at least one row is selected.
  • Unit tests with proper use cases for indeterminate states.

Test Steps

  • ng serve
  • Go to data-table demo
  • See indeterminate state when selecting one row in multiple/selectable demo

General Tests for Every PR

  • ng serve --aot still works.
  • npm run lint passes.
  • npm test passes and code coverage is not lower.
  • npm run build still works.
Screenshots or link to CodePen/Plunker/JSfiddle

dt-indeterminate

when atleast one row is selected and not every row is selected, the checkbox for select/deselect all should be in indeterminate state.
@emoralesb05 emoralesb05 added this to the Beta 4 milestone May 8, 2017
@jeremysmartt jeremysmartt self-assigned this May 8, 2017
@@ -303,6 +312,7 @@ export class TdDataTableComponent implements ControlValueAccessor, AfterContentI
} else {
this.clearModel();
}
this._calculateCheckboxState();
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not just set the this._allSelected here? You already looped through everything so you shouldn't need to do it again as you already know what the state should be.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

true! ill change that real quick

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated~

})();
});

it('should select one and be in indeterminate state, select all and then unselect all',
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice unit test 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🍷 🎩

@kyleledbetter
Copy link
Contributor

UX is lovely, i'm ready to merge when i get signoff on the code

@kyleledbetter kyleledbetter merged commit bd0f7bc into develop May 8, 2017
@kyleledbetter kyleledbetter deleted the feature/dt-indet-state branch May 8, 2017 23:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants