Issue 370 - Clearing filtering_settings clears the by-column filter…

… UI (#372)
plotly · Feb 19, 2019 · e144584 · e144584
1 parent f3e5e3f
commit e144584
Show file tree

Hide file tree

Showing 12 changed files with 137 additions and 52 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,9 @@ This project adheres to [Semantic Versioning](http://semver.org/).
     - `style_header_conditional`
 
 ### Fixed
+[#347](https://github.com/plotly/dash-core/issues/347)
+- Fixed table behavior when `filtering_settings` is updated through a callback
+
 [#322](https://github.com/plotly/dash-core/issues/322)
 - Added LICENSE file to Python distributable
 

diff --git a/demo/App.js b/demo/App.js
@@ -2,17 +2,18 @@
 import * as R from 'ramda';
 import React, {Component} from 'react';
 import { DataTable } from 'dash-table';
+import Environment from 'core/environment';
 import { memoizeOne } from 'core/memoizer';
 import Logger from 'core/Logger';
-import AppMode from './AppMode';
+import AppState, { AppMode } from './AppMode';
 
 import './style.less';
 
 class App extends Component {
     constructor() {
         super();
 
-        this.state = AppMode;
+        this.state = AppState;
 
         const setProps = memoizeOne(() => {
             return newProps => {
@@ -28,11 +29,30 @@ class App extends Component {
         });
     }
 
+    renderMode() {
+        const mode = Environment.searchParams.get('mode');
+
+        if (mode === AppMode.Filtering) {
+            return (<button
+                className='clear-filters'
+                onClick={() => {
+                    const tableProps = R.clone(this.state.tableProps);
+                    tableProps.filtering_settings = '';
+
+                    this.setState({ tableProps });
+                }}
+            >Clear Filter</button>);
+        }
+    }
+
     render() {
-        return (<DataTable
-            setProps={this.setProps}
-            {...this.state.tableProps}
-        />);
+        return (<div>
+            {this.renderMode()}
+            <DataTable
+                setProps={this.setProps}
+                {...this.state.tableProps}
+            />
+        </div>);
     }
 }
 

diff --git a/demo/AppMode.ts b/demo/AppMode.ts
@@ -16,6 +16,7 @@ import { TooltipSyntax } from 'dash-table/tooltips/props';
 export enum AppMode {
     Date = 'date',
     Default = 'default',
+    Filtering = 'filtering',
     FixedTooltips = 'fixed,tooltips',
     FixedVirtualized = 'fixed,virtualized',
     ReadOnly = 'readonly',
@@ -31,6 +32,11 @@ export const ReadWriteModes = [
     AppMode.Virtualized
 ];
 
+export const BasicModes = [
+    ...ReadWriteModes,
+    AppMode.ReadOnly
+];
+
 function getBaseTableProps(mock: IDataMock) {
     return {
         id: 'table',
@@ -209,6 +215,13 @@ function getDateState() {
     return state;
 }
 
+function getFilteringState() {
+    const state = getDefaultState();
+    state.tableProps.filtering = true;
+
+    return state;
+}
+
 function getVirtualizedState() {
     const mock = generateMockData(5000);
 
@@ -253,6 +266,8 @@ function getState() {
     switch (mode) {
         case AppMode.Date:
             return getDateState();
+        case AppMode.Filtering:
+            return getFilteringState();
         case AppMode.FixedTooltips:
             return getFixedTooltipsState();
         case AppMode.FixedVirtualized:

diff --git a/src/core/components/IsolatedInput/index.tsx b/src/core/components/IsolatedInput/index.tsx
@@ -38,6 +38,16 @@ export default class IsolatedInput extends PureComponent<IProps, IState> {
         return this.props as PropsWithDefaults;
     }
 
+    componentWillReceiveProps(nextProps: IProps) {
+        const { value: nextValue } = nextProps;
+
+        if (this.state.value !== nextValue) {
+            this.setState({
+                value: nextValue
+            });
+        }
+    }
+
     constructor(props: PropsWithDefaults) {
         super(props);
 

diff --git a/src/core/syntax-tree/index.ts b/src/core/syntax-tree/index.ts
@@ -18,18 +18,16 @@ export default class SyntaxTree {
     }
 
     evaluate = (target: any) => {
-        if (!this.isValid || !this.tree) {
+        if (!this.isValid) {
             const msg = `unable to evaluate target: syntax tree is invalid for query=${this.query}`;
 
             Logger.error(msg);
             throw new Error(msg);
         }
 
-        const evaluate = this.tree.lexeme.evaluate;
-
-        return evaluate ?
-            evaluate(target, this.tree) :
-            false;
+        return this.tree && this.tree.lexeme && this.tree.lexeme.evaluate ?
+            this.tree.lexeme.evaluate(target, this.tree) :
+            true;
     }
 
     filter = (targets: any[]) => {

diff --git a/src/core/syntax-tree/syntaxer.ts b/src/core/syntax-tree/syntaxer.ts
@@ -64,6 +64,10 @@ export default (lexerResult: ILexerResult): ISyntaxerResult => {
         return { valid: false, error: `lexer -- ${lexerResult.error}` };
     }
 
+    if (lexerResult.lexemes.length === 0) {
+        return { valid: true };
+    }
+
     try {
         return { tree: parser(lexemes), valid: true };
     } catch (error) {

diff --git a/src/dash-table/components/Filter/Column.tsx b/src/dash-table/components/Filter/Column.tsx
@@ -14,11 +14,7 @@ interface IColumnFilterProps {
     value?: string;
 }
 
-interface IColumnFilterState {
-    value?: string;
-}
-
-export default class ColumnFilter extends PureComponent<IColumnFilterProps, IColumnFilterState> {
+export default class ColumnFilter extends PureComponent<IColumnFilterProps> {
     constructor(props: IColumnFilterProps) {
         super(props);
 
@@ -27,16 +23,6 @@ export default class ColumnFilter extends PureComponent<IColumnFilterProps, ICol
         };
     }
 
-    componentWillReceiveProps(nextProps: IColumnFilterProps) {
-        const { value: nextValue } = nextProps;
-
-        if (this.state.value !== nextValue) {
-            this.setState({
-                value: nextValue
-            });
-        }
-    }
-
     private submit = (value: string | undefined) => {
         const { setFilter } = this.props;
 

diff --git a/src/dash-table/components/FilterFactory.tsx b/src/dash-table/components/FilterFactory.tsx
@@ -130,8 +130,12 @@ export default class FilterFactory {
         }
 
         const { tree } = syntaxerResult;
-        const toCheck: (ISyntaxTree | undefined)[] = [tree];
+        if (!tree) {
+            this.ops.clear();
+            return;
+        }
 
+        const toCheck: (ISyntaxTree | undefined)[] = [tree];
         while (toCheck.length) {
             const item = toCheck.pop();
             if (!item) {

diff --git a/tests/cypress/tests/standalone/filtering_test.ts b/tests/cypress/tests/standalone/filtering_test.ts
@@ -5,31 +5,76 @@ import Key from 'cypress/Key';
 import { AppMode } from 'demo/AppMode';
 
 describe(`filter`, () => {
-    beforeEach(() => {
-        cy.visit(`http://localhost:8080?mode=${AppMode.ColumnsInSpace}`);
-        DashTable.toggleScroll(false);
+    describe(`special characters`, () => {
+        beforeEach(() => {
+            cy.visit(`http://localhost:8080?mode=${AppMode.ColumnsInSpace}`);
+            DashTable.toggleScroll(false);
+        });
+
+        it('can filter on special column id', () => {
+            DashTable.getFilterById('c cc').click();
+            DOM.focused.type(`gt num(90)${Key.Enter}`);
+
+            DashTable.getFilterById('d:dd').click();
+            DOM.focused.type(`lt num(12500)${Key.Enter}`);
+
+            DashTable.getFilterById('e-ee').click();
+            DOM.focused.type(`is prime${Key.Enter}`);
+
+            DashTable.getFilterById('f_ff').click();
+            DOM.focused.type(`le num(106)${Key.Enter}`);
+
+            DashTable.getFilterById('g.gg').click();
+            DOM.focused.type(`gt num(1000)${Key.Enter}`);
+
+            DashTable.getFilterById('b+bb').click();
+            DOM.focused.type(`eq "Wet"${Key.Enter}`);
+
+            DashTable.getCellById(0, 'rows').within(() => cy.get('.dash-cell-value').should('have.html', '101'));
+            DashTable.getCellById(1, 'rows').should('not.exist');
+        });
     });
 
-    it('can filter on special column id', () => {
-        DashTable.getFilterById('c cc').click();
-        DOM.focused.type(`gt num(90)${Key.Enter}`);
+    describe('reset', () => {
+        beforeEach(() => {
+            cy.visit(`http://localhost:8080?mode=${AppMode.Filtering}`);
+            DashTable.toggleScroll(false);
+        });
+
+        it('updates results and filter fields', () => {
+            let cell_0;
+            let cell_1;
+
+            DashTable.getCellById(0, 'ccc')
+                .within(() => cy.get('.dash-cell-value')
+                    .then($el => cell_0 = $el[0].innerHTML));
 
-        DashTable.getFilterById('d:dd').click();
-        DOM.focused.type(`lt num(12500)${Key.Enter}`);
+            DashTable.getCellById(1, 'ccc')
+                .within(() => cy.get('.dash-cell-value')
+                    .then($el => cell_1 = $el[0].innerHTML));
 
-        DashTable.getFilterById('e-ee').click();
-        DOM.focused.type(`is prime${Key.Enter}`);
+            DashTable.getFilterById('ccc').click();
+            DOM.focused.type(`gt num(100)`);
+            DashTable.getFilterById('ddd').click();
+            DOM.focused.type('lt num(20000)');
+            DashTable.getFilterById('eee').click();
+            DOM.focused.type('is prime');
+            DashTable.getFilterById('bbb').click();
+            DOM.focused.type(`eq "Wet"`);
+            DashTable.getFilterById('ccc').click();
 
-        DashTable.getFilterById('f_ff').click();
-        DOM.focused.type(`le num(106)${Key.Enter}`);
+            DashTable.getCellById(0, 'ccc').within(() => cy.get('.dash-cell-value').should('have.html', '101'));
+            DashTable.getCellById(1, 'ccc').within(() => cy.get('.dash-cell-value').should('have.html', '109'));
 
-        DashTable.getFilterById('g.gg').click();
-        DOM.focused.type(`gt num(1000)${Key.Enter}`);
+            cy.get('.clear-filters').click();
 
-        DashTable.getFilterById('b+bb').click();
-        DOM.focused.type(`eq "Wet"${Key.Enter}`);
+            DashTable.getCellById(0, 'ccc').within(() => cy.get('.dash-cell-value').should('have.html', cell_0));
+            DashTable.getCellById(1, 'ccc').within(() => cy.get('.dash-cell-value').should('have.html', cell_1));
 
-        DashTable.getCellById(0, 'rows').within(() => cy.get('.dash-cell-value').should('have.html', '101'));
-        DashTable.getCellById(1, 'rows').should('not.exist');
+            DashTable.getFilterById('bbb').within(() => cy.get('input').should('have.value', ''));
+            DashTable.getFilterById('ccc').within(() => cy.get('input').should('have.value', ''));
+            DashTable.getFilterById('ddd').within(() => cy.get('input').should('have.value', ''));
+            DashTable.getFilterById('eee').within(() => cy.get('input').should('have.value', ''));
+        });
     });
 });
diff --git a/tests/cypress/tests/standalone/navigation_test.ts b/tests/cypress/tests/standalone/navigation_test.ts
@@ -2,9 +2,9 @@ import DashTable from 'cypress/DashTable';
 import DOM from 'cypress/DOM';
 import Key from 'cypress/Key';
 
-import { AppMode } from 'demo/AppMode';
+import { BasicModes } from 'demo/AppMode';
 
-Object.values(AppMode).forEach(mode => {
+Object.values(BasicModes).forEach(mode => {
     describe(`navigate, mode=${mode}`, () => {
         beforeEach(() => {
             cy.visit(`http://localhost:8080?mode=${mode}`);

diff --git a/tests/cypress/tests/standalone/select_row_test.ts b/tests/cypress/tests/standalone/select_row_test.ts
@@ -1,8 +1,8 @@
 import DashTable from 'cypress/DashTable';
 
-import { AppMode } from 'demo/AppMode';
+import { BasicModes } from 'demo/AppMode';
 
-Object.values(AppMode).forEach(mode => {
+Object.values(BasicModes).forEach(mode => {
     describe(`select row, mode=${mode}`, () => {
         beforeEach(() => {
             cy.visit(`http://localhost:8080?mode=${mode}`);

diff --git a/tests/cypress/tests/standalone/selection_test.ts b/tests/cypress/tests/standalone/selection_test.ts
@@ -2,9 +2,9 @@ import DashTable from 'cypress/DashTable';
 import DOM from 'cypress/DOM';
 import Key from 'cypress/Key';
 
-import { AppMode } from 'demo/AppMode';
+import { BasicModes } from 'demo/AppMode';
 
-Object.values(AppMode).forEach(mode => {
+Object.values(BasicModes).forEach(mode => {
     describe(`select, mode=${mode}`, () => {
         beforeEach(() => {
             cy.visit(`http://localhost:8080?mode=${mode}`);