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: Searchable and Unsearchable columns #548

Merged
merged 13 commits into from
May 5, 2017

Conversation

jeremysmartt
Copy link
Collaborator

@jeremysmartt jeremysmartt commented May 3, 2017

Description

Added notsearchable property on columns to keep certain columns from being searched on in the filterData method. Also added unit tests for table and updated documentation.

Closes #513

What's included?

  • modified: src/app/components/components/data-table/data-table.component.html
  • modified: src/app/components/components/data-table/data-table.component.ts
  • modified: src/platform/core/data-table/data-table.component.ts
  • modified: src/platform/core/data-table/services/data-table.service.ts
  • new file: src/platform/core/data-table/data-table.component.spec.ts

Test Steps

  • checkout branch
  • Go to http://localhost:4200/#/components/data-table
  • Go to the "Data Table with components"
  • Click the Search magnify glass
  • type in 392 see no results
  • delete 392 from search box
  • Click the TOGGLE SEARCHING CALORIES COLUMN button
  • See 1 result row in the table
  • Make sure documentation makes sense

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

non-searchable-columns

Added notsearchable property on columns to keep certain columns from being searched on in the filterData method. Also added unit tests for table and updated documentation.

Closes #513
@jeremysmartt jeremysmartt changed the title feat: Data Table: Searchable and unsearchable columns feat: Data Table: Searchable and Unsearchable columns May 3, 2017
@kyleledbetter
Copy link
Contributor

love the PR, could we name the input something more simple like filter="false" and true is the default?

@emoralesb05
Copy link
Contributor

emoralesb05 commented May 3, 2017

I feel that this is more a design pattern than an enhancement.. not sure about this yet. Although i do see the need for that extra parameter in TdDataTableService

@jeremysmartt
Copy link
Collaborator Author

Right at first I thought maybe a design pattern, but then actually I don't think it is. I think the ability to pass in the columns and get hits based on properties will continue to be needed. For example, probably want to do the same thing when a column is hidden:
#511

@emoralesb05
Copy link
Contributor

The use case is legit, just debating if it should be part of the interface or applied in a different way since this seems more of an sorting implementation thing.

In both cases, there is stuff to be done in TdDataTableService for sure since thats a sorting implementation.

@jeremysmartt
Copy link
Collaborator Author

@kyleledbetter I updated the name of the input to be filter="false" and true is the default.

@kyleledbetter
Copy link
Contributor

since we have a new batch of filtering/hiding/showing features coming, thinking we can do a new demo since that current demo is getting a little convoluted

@kyleledbetter
Copy link
Contributor

maybe not another demo, but maybe update the current complex datatable demo to use slide toggles for options instead of md-buttons, i'll take a stab

@thomaslennan
Copy link
Collaborator

Another framework provides the ability to define both global and individual filters. Wondering if this PR should really be addressed by being able to filter on a per column basis. I, as a user, would find it confusing that I saw data in a column of the table that wasn't visible after input of some global filter value that matched the value that saw in that column. Alternatively, I'd want some visual indicator that the particular column isn't participating in the filter?

KL186023 and others added 5 commits May 4, 2017 14:41
…om the search rather than the full columns config object. This will allow the columns by which you are searching on to be more easily changed on the fly
@@ -13,14 +13,16 @@ export class TdDataTableService {
*
* Searches [data] parameter for [searchTerm] matches and returns a new array with them.
*/
filterData(data: any[], searchTerm: string, ignoreCase: boolean = false): any[] {
filterData(data: any[], searchTerm: string, ignoreCase: boolean = false, nonSearchAbleColumns?: string[]): any[] {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we just call this excludedColumns?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated

import { NgModule, DebugElement } from '@angular/core';
import { BrowserAnimationsModule } from '@angular/platform-browser/animations';

describe('Component: TdPagingBarComponent', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

should be 'Component: DataTable'.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated

@@ -328,7 +328,7 @@ <h4 class="md-subhead">Paging Bar / Search Box / Sortable components</h4>

filter(): void {
let newData: any[] = this.data;
newData = this._dataTableService.filterData(newData, this.searchTerm, true);
newData = this._dataTableService.filterData(newData, this.searchTerm, true, this.columns);
Copy link
Contributor

Choose a reason for hiding this comment

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

Demo needs to be updated with latest API changes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated

…n unit test file, updating demo with latest API changes
@emoralesb05 emoralesb05 merged commit 11c3d15 into develop May 5, 2017
@emoralesb05 emoralesb05 deleted the feature/unsearchable-columns branch May 5, 2017 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants