Skip to content
This repository has been archived by the owner on Jun 4, 2024. It is now read-only.

Issue 224 - Improve column id support for filtering #359

Merged
merged 8 commits into from
Feb 5, 2019

Conversation

Marc-Andre-Rivet
Copy link
Contributor

@Marc-Andre-Rivet Marc-Andre-Rivet commented Feb 4, 2019

Fixes #224.

Many normal usage scenarios are calling for column ids with non-word [^a-zA-Z0-9_] characters.
This PR adds the needed support for a subset of special characters [-+.:] without quotes, and for the full set of characters when single or double quoted.

Adds a few unit tests for the syntactic tree and a standalone/integration test for real-world usage validation.

Tests are passing against dash/dash-renderer latest release

Marc-André Rivet added 2 commits February 4, 2019 16:29
- clean up lexicon selector
- arbitrary id test
@chriddyp chriddyp temporarily deployed to dash-table-review-pr-359 February 4, 2019 21:45 Inactive
@@ -202,6 +212,8 @@ function getState() {
return getFixedVirtualizedState();
case AppMode.ReadOnly:
return getReadonlyState();
case AppMode.ColumnsInSpace:
return getSpaceInColumn();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A new mock/mode with specially named columns

@@ -55,7 +55,7 @@ export default class FilterFactory {
}

setFilter(R.map(
([cId, filter]) => `${cId} ${filter}`,
([cId, filter]) => `"${cId}" ${filter}`,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Auto-quote the column id when generating the filter fragments from column filters


static getFilterById(column: number | string) {
return cy.get(`#table tbody tr th.dash-filter[data-dash-column="${column}"]`);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

New selectors for getting the filter cells


DashTable.getCellById(0, 'rows').within(() => cy.get('.dash-cell-value').should('have.html', '101'));
DashTable.getCellById(1, 'rows').should('not.exist');
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

test that the filters are working fine for various special cases

@chriddyp chriddyp temporarily deployed to dash-table-review-pr-359 February 4, 2019 21:53 Inactive
@chriddyp chriddyp temporarily deployed to dash-table-review-pr-359 February 4, 2019 22:07 Inactive
@chriddyp chriddyp temporarily deployed to dash-table-review-pr-359 February 4, 2019 22:14 Inactive
- test nested quotes
- add `` to '' and "" support
@chriddyp chriddyp temporarily deployed to dash-table-review-pr-359 February 5, 2019 17:02 Inactive
- add tests for badly formed operands
@chriddyp chriddyp temporarily deployed to dash-table-review-pr-359 February 5, 2019 23:13 Inactive
Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

Looks great! 💃

@Marc-Andre-Rivet Marc-Andre-Rivet merged commit 22d3bba into master Feb 5, 2019
@Marc-Andre-Rivet Marc-Andre-Rivet deleted the issue224-column-name branch July 18, 2019 12:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants