[Vis: Default editor] EUIficate filters control

[sulemanof]

Summary

Part of #30922.
EUIfication of filters control for Filters aggregation parameter.

The previous UI:

The current implementation:

UI changes:

• used EUI icons;
• added a plus icon into Add Filter button;
• remove filter button disabled if it is the only filter

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

• This was checked for cross-browser compatibility, including a check against IE11
• Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
• Documentation was added for features that require explanation or tutorials
• Unit or functional tests were updated or added to match the most common scenarios
• This was checked for keyboard-only and screenreader accessibility

For maintainers

• This was checked for breaking API changes and was labeled appropriately
• This includes a feature addition or change that requires a release note and was labeled appropriately

[sulemanof]

@cchaos could you please take a look at the current design of the filters component and provide your feedback 😊

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[cchaos]

I've been trying to think of ways to improve this UI and the only thing I can come up with would require a bit more work. It would be really nice if these filters could re-use the popover that exists in the global filter bar and then display similarly like so:

---

Otherwise I don't think there's much we can do to enhance this UI. Thoughts on the possibility of accomplishing the above?

[maryia-lapata]

Thoughts on the possibility of accomplishing the above?

@cchaos looking briefly, we can reuse the popover. That FilterEditor provides ample opportunity (field and operator selector, value, range) and I'm not sure whether we need them for the visualization editor, also it seems for me as a kind of duplication of the global filter bar.
Also we need to investigate how to pass all configuration from FilterEditor to visualization API, how to proper convert it. Probably @timroes has wider vision about the possibility of that.

[cchaos]

Maybe if not the actual popover from the global filters, but a similar pattern? So instead of writing out the filters inline in the sidebar, you open a popover with a text editor (similar to the one in "Edit as Query DSL") and then use the filter pill format to display and remove them?

[timroes]

@cchaos we should for now leave that as simple text boxes, since a filter is something completely different than a query (which you ironically put into the filter aggregation :D). Changing the filter aggregation to use the filter editor (and thus also need to use the filter to query DSL builder (and not a simple lucene query anymore) is a significant rewrite, which goes beyond this PR. Also one filter in the filter aggregation (since being a Lucene string) usually contains multiple filter queries, so basically each filter in a filter aggregation would not be represented by one filter pill/editor but by one filter bar itself. (And also we would need a new filter bar, which supports AND and OR for filters beforehand :D)

But please feel free to create a separate issue for this, since @Bargs will anyway be looking into supporting KQL for the filter aggregation. Maybe during that, we come closer with the infrastructure to also use a more UI wise approach with the filter editor.

[cchaos]

If nothing can be done to improve the UI at this point, then from a straight angular to React (EUI) conversion, this is fine.

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[elasticmachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request

[Bargs]

I'd love to see this get merged as soon as possible. As we speak I'm in the middle of converting these inputs into query bars that support both lucene and KQL as well as autocomplete. Having the editor in React will solve one of the issues I'm currently facing. So the sooner this gets merged the sooner I can continue to progress on that work.

I did a review of the code and functionality and everything LGTM!

[elasticmachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request

[elasticmachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request

[Bargs]

LGTM

[Bargs]

@sulemanof is this good to merge?

[Bargs]

Since this is blocking me I'm going to go ahead and merge it considering the tests have passed and it's been reviewed.

[sulemanof]

Hi @Bargs ! Sorry for late reply, I was on vacation.
I hope everything is good in this PR and it was ready for merge.
We also have to backport it to 7.x branch

[elasticmachine]

Pinging @elastic/kibana-app

[lizozom]

@sulemanof @Bargs
The propFilter dependency used in vis_type_agg_filter, needs to be removed.
App Arch team is about to delete the filters directory, and that's the only thing remaining in it.
Maybe it can be simplified for your use case and hosted in this file?
In the worst case, please copy it into your code.

[sulemanof]

The propFilter dependency used in vis_type_agg_filter, needs to be removed.

If we need to remove the propFilter we have to create a separate PR for that stuff because it has not any relation to the Filter Aggregation (which the existing pr consists of)

@timroes what should we do with that?

[timroes]

@sulemanof Yes, I agree with Liza here. Let's move that code into the vis_type_agg_filter directly. Also let's shorten it for what we need, so we can basically remove the wrapping function and just use the inner function on the property name directly (will also be a bit more typesafe).

@sulemanof @maryia-lapata could either of you open a PR for that?

[maryia-lapata]

@timroes sure, I'll do it.

[maryia-lapata]

@timroes propFilter is used in two places, so I just moved it to ui/agg_types/filter #36875

cc/ @lizozom