search

GriddleGriddle / Griddle

Simple Grid Component written in React
http://griddlegriddle.github.io/Griddle/
MIT License
2.5k stars 378 forks source link

Stop filter from matching invisible properties #800

Closed mbland closed 6 years ago

mbland commented 6 years ago

Griddle major version

1

Changes proposed in this pull request

Closes #725. This fixes the problem of invisible fields getting matched by the filter by updating the filteredDataSelector() condition to exclude griddleKey and any values not covered by a ColumnDefinition if any ColumnDefinitions are present.

Bear in mind that columns corresponding to other React components may still match the filter based on generated HTML properties rendered as part of the value.toString().toLowerCase() expression. At this point the best course of action may be to recommend that such ColumnDefinitions are tagged with filterable={false} if it's a significant problem for the user.

Why these changes are made

The previous if (filterable === false) condition meant that filterable={false} must be explicitly set on a ColumnDefinition in order for any property returned by row.keySeq() to be ignored. This resulted in the filter matching invisible row properties such as griddleKey and any properties in the data not covered by a ColumnDefinition.

I confirmed this by locally changing the expression at the end of this filter from:

  return value &&
    value.toString().toLowerCase().indexOf(filterToLower) > -1;

to:

  if (value && value.toString().toLowerCase().indexOf(filterToLower) > -1) {
    console.log('KEY:', key, 'VALUE:', value.toString().toLowerCase());
    return true;
  }
  return false;

and observed the unexpected filter matches on invisible row properties in the console.

Are there tests?

I've added updated existing tests, added new tests, and validated the filter behavior in the Storybook.

dahlbyk commented 6 years ago

💯