Skip to content

Invalid filters#375

Merged
texodus merged 15 commits intoperspective-dev:masterfrom
Ro4052:invalid-filters
Jan 14, 2019
Merged

Invalid filters#375
texodus merged 15 commits intoperspective-dev:masterfrom
Ro4052:invalid-filters

Conversation

@Ro4052
Copy link
Contributor

@Ro4052 Ro4052 commented Jan 10, 2019

#349 UI filters should give user feedback when they are valid

  • Exposed new table method for determining if a filter is valid
  • Viewer validates filters before passing to table
  • Invalid filters operands are underlined in red with a red exclamation mark next to them
  • Filtering by an empty string is now invalid (this seems more appropriate for the current viewer UI)
  • Added a screenshot test for the new invalid filter highlighting
  • Added test for filtering by empty string
  • Added tests for newly exposed is_valid_filter method

invalid_filter_exclamation

@timkpaine timkpaine added enhancement Feature requests or improvements JS labels Jan 10, 2019
@texodus
Copy link
Member

texodus commented Jan 14, 2019

Thanks for the PR! This is a great addition - User experience is awesome, and the tests in particular are greatly appreciated!

I think as a future enhancement we may want to reconsider this API, in favor of general validation across APIs which take configuration objects as parameters - see also my comments on API differences between viewer and library. I think this will help keep the API surface small, predictable, as well as portable and consistent between host languages.

As this is currently undocumented, IMO it is fine as is for now.

@texodus texodus merged commit df251b9 into perspective-dev:master Jan 14, 2019
@Ro4052 Ro4052 deleted the invalid-filters branch January 15, 2019 09:18
@Ro4052 Ro4052 mentioned this pull request Jan 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Feature requests or improvements JS

Development

Successfully merging this pull request may close these issues.

4 participants