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

Feature: Advanced Filter (Regex Matching, Case Matching, Full Words Matching, Tag/Route) #6744

Closed

Conversation

mathis-m
Copy link
Contributor

@mathis-m mathis-m commented Dec 28, 2020

Description

In order to reflect common filtering, like we all know from IDE's, there is the need for new filter configuration options.
It need to store the 3 Flags: regex-, case-, full words-mode and the search location (tag or route/path).
In order to reflect the old behavior defaults are like the following:

filterConfig: {
  isRegexFilter: false,
  matchCase: true,
  matchWords: false,
  searchLocation: "tag" // or "route"
}

Have a look at the preview.

Open for more filter features, let's discuss this in #6753.

Motivation and Context

Reflect common filtering, like we all know from IDE's.
Fixes #6648

How Has This Been Tested?

Fixed suts of tests using the filter logic to provide the currentFilterConfig.
Wrote tests for new filter algorithm src/core/plugins/filter/opsFilter.js.

Checklist

My PR contains...

  • No code changes (src/ is unmodified: changes to documentation, CI, metadata, etc.)
  • Dependency changes (any modification to dependencies in package.json)
  • Bug fixes (non-breaking change which fixes an issue)
  • Improvements (misc. changes to existing features)
  • Features (non-breaking change which adds functionality)

My changes...

  • are breaking changes to a public API (config options, System API, major UI change, etc).
  • are breaking changes to a private API (Redux, component props, utility functions, etc.).
  • are breaking changes to a developer API (npm script behavior changes, new dev system dependencies, etc).
  • are not breaking changes.

Documentation

  • My changes do not require a change to the project documentation.
  • My changes require a change to the project documentation.
  • If yes to above: I have updated the documentation accordingly.

Automated tests

  • My changes can not or do not need to be tested.
  • My changes can and should be tested by unit and/or integration tests.
  • If yes to above: I have added tests to cover my changes.
  • If yes to above: I have taken care to cover edge cases in my tests.
  • All new and existing tests passed.

@mathis-m mathis-m force-pushed the feature/filter-configuration branch 3 times, most recently from 672a025 to 6458623 Compare December 29, 2020 22:59
@mathis-m mathis-m force-pushed the feature/filter-configuration branch from 6458623 to 575bc62 Compare December 30, 2020 01:08
added select to switch between tag and route search
@mathis-m mathis-m changed the title Feature: Advanced Filter (Regex Matching, Case Matching, Full Words Matching) Feature: Advanced Filter (Regex Matching, Case Matching, Full Words Matching, Tag/Route) Dec 30, 2020
@mathis-m mathis-m marked this pull request as draft January 8, 2021 02:58
@tim-lai
Copy link
Contributor

tim-lai commented Jan 12, 2021

Hi @mathis-m, this PR and demo looks very promising. A couple of thoughts:

  1. I think "match whole word", "match case exactly", and the combination of the two, is a great place to start.
  2. I think user input of regex search expression is a nice to have.
  3. The text filter by default should apply to all.
  4. We may want to apply the text filter to tag, operation, as well as future categories, so there should be a separate "category" filter (needs a better descriptor), with its own configuration, that we could chain a call. The idea is that a "category" filter could be extended (as plugin?) by others in the future.
  5. The UI component to set/unset the text filter options should be in its own component, and abled to be placed "anywhere". I've seen two filter input sizes to be aware of. The first is "wide", which is current swagger-ui default full width, The second is "narrow", the width of an arbitrary sidebar.
  6. We should mostly leave the existing opsFilter as-is, perhaps reference it as the "simple" filter. We can then offer an "advancedFilter" option, with sub-options proposed in this PR, and as separate method(s). This should help existing customized versions of swagger-ui to adopt and migrate when ready, as well as enable for additional and/or extended filters.

Finally, expect that this will need a UX/UI review once we get through the functionality implementation.

@mathis-m
Copy link
Contributor Author

Hi @tim-lai,

thanks for your Feedback and I completely agree on all your points.
I will open another PR with the updated code soon.

@mathis-m
Copy link
Contributor Author

closing in favor of #6851

@mathis-m mathis-m closed this Jan 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: whole word filter option
2 participants