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

[Controls] Wildcard Search for Options List #157157

Closed
ThomThomson opened this issue May 9, 2023 · 1 comment · Fixed by #158427
Closed

[Controls] Wildcard Search for Options List #157157

ThomThomson opened this issue May 9, 2023 · 1 comment · Fixed by #158427
Assignees
Labels
Feature:Input Control Input controls visualization impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:large Large Level of Effort regression Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas

Comments

@ThomThomson
Copy link
Contributor

Problem
The Legacy Input Controls used a terms include query with a wildcard. This allowed the legacy controls to search for terms using any part of the string. Instead, the new controls use a prefix query.

This blocks us from fully deprecating the legacy input controls, because we cannot remove this functionality.

How to reproduce

  1. Install any sample data set
  2. Create a dashboard with a legacy options list and an options list control
  3. In the options list control, search for some non-prefix part of the term, metricbeat in message.keyword in the example below, and see that no options are available.
    image_720
  4. In the legacy options list control, search for some non-prefix part of the term, metricbeat in message.keyword in the example below, and see that options are available.
    image_720-1

Potential Solutions
To fix this, we will likely need to introduce a switch into the options list config called wildcard search or something similar. With the wildcard setting on, we could either:

  • switch from the prefix query to a wildcard query. This has the benefits of preserving the case-insensitive search, and the cardinality, but we may need to research how feasible it is from a behaviour and performance perspective.
  • Use the terms include query from the legacy controls. This would ensure that the behaviour remains the same between the new and old controls, but it would mean that we could no longer show cardinality on search, and that with this setting on, search would be case-sensitive again.
@ThomThomson ThomThomson added regression Feature:Input Control Input controls visualization Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas loe:large Large Level of Effort impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. labels May 9, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@Heenawter Heenawter self-assigned this May 23, 2023
@ThomThomson ThomThomson changed the title [Controls] Wildcard Search for Options List [SDH][Controls] Wildcard Search for Options List May 23, 2023
@Heenawter Heenawter changed the title [SDH][Controls] Wildcard Search for Options List [Controls] Wildcard Search for Options List May 23, 2023
Heenawter added a commit that referenced this issue Jun 6, 2023
Closes #157157
Closes #152921 

## Summary

The primary goal of this PR is to introduce an option for wildcard
("contains") searching to the options list control:



https://github.com/elastic/kibana/assets/8698078/6847d0c5-014a-4322-8e59-308bc3ca27aa

### New Flyout Design

However, since this required adding a radio group to the custom options
list settings in the create/edit control flyout, I also made some
changes to the flyout design in order to better accommodate this (we
were previously using `EuiSwitch` components for the
control-type-specific settings, which did not work in this case because
I wanted to be able to add a tooltip to describe each search type):

| Before | After |
|--------|-------|
| ![May-29-2023
09-04-02](https://github.com/elastic/kibana/assets/8698078/e7a8dba8-0815-4460-b55a-f622300e40ac)
| ![May-29-2023
09-00-21](https://github.com/elastic/kibana/assets/8698078/50fa6795-f7e1-4329-bac6-045f6f8464b3)
|

Note in the above GIFs that, since I was using an `EuiRadioGroup` for
the search technique setting, I decided it made more sense + was more
consistent for the "Allow multiple selections in dropdown" to also be
converted to a radio group rather than a switch. The "Ignore timeout for
results" setting is the only one that remained a switch in the new
design:

| Before | After |
|--------|-------|
|
![image](https://github.com/elastic/kibana/assets/8698078/53b1168c-b172-4cd0-8fbf-ce80a03964c0)
|
![image](https://github.com/elastic/kibana/assets/8698078/60b174a6-635f-4b09-8f1a-fef1b0e7bb2c)
|


### `EuiSwitch` with Tooltip Bug

As part of this redesign, I also fixed a very quick bug where, because
the old `SwitchWithTooltip` was defined **inside** the larger
`OptionsListEditorOptions` component, any state update on
`OptionsListEditorOptions` would cause `SwitchWithTooltip` to **also**
be re-rendered - this caused the "slide" animation to be interrupted on
click:

| Before | After |
|--------|-------|
|
![image](https://github.com/elastic/kibana/assets/8698078/03bebc7c-b529-463e-a042-7aa680c99eeb)
|
![image](https://github.com/elastic/kibana/assets/8698078/db0fe6e7-d352-476a-8b77-d93d835c9942)
|




### Title Bug Fix
And, since I was making so many changes to the flyout code as part of
refactoring the code (including the design changes above), I also fixed
a bug with control titles where things weren't getting set properly. To
test this, consider taking the following steps:
1. Create a new options list control, keeping the default title
2. Edit that options list control and change it to a range slider
control by selecting a number field
3. Notice that...
     - Before this PR, the title gets completely cleared:

![image](https://github.com/elastic/kibana/assets/8698078/cdd9ecc2-c729-402c-8f69-018de3e60342)<br>
- After this PR, the default title gets updated to the range slider
field name:

![image](https://github.com/elastic/kibana/assets/8698078/d56ec9f9-6961-4799-ba3b-623ae5f46d91)<br>
4. Delete that range slider control and create a new control, keeping
the default title once again (options list or range slider, the type
doesn't matter).
5. Edit the control and change the field to a different field **of the
same type** (i.e. if your control from step 4 was an options list
control, select a field that keeps it as an options list control).
Before saving your changes, notice that the "default title" in the
`Label` input gets updated to the new field title:<br>
<img width="450"
src="https://github.com/elastic/kibana/assets/8698078/0fabe2e3-7f83-4f2a-87e6-33253652972d"/><br>
6. After saving, notice that...
- Before this PR, the title doesn't actually get updated to the new
default title:

![image](https://github.com/elastic/kibana/assets/8698078/b9262c67-9841-47f0-8d25-d5689fe408de)<br>
     - After this PR, the title gets updated as expected:

![image](https://github.com/elastic/kibana/assets/8698078/1a2b3f19-a02c-4525-9d2c-8029c020a117)<br>

### Flaky Test Runner


[test/functional/apps/dashboard_elements/controls/options_list/options_list_suggestions.ts](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2343):
<img
src="https://github.com/elastic/kibana/assets/8698078/f2ed9d65-adcf-47af-bb00-ee11837c406b"/>

### Checklist

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [x] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [x] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [x] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)


### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Input Control Input controls visualization impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:large Large Level of Effort regression Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants