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

[Search Bar/Query/Default Syntax] Parser support for "recognizedFields" options #7960

Merged
merged 9 commits into from
Aug 22, 2024

Conversation

tsullivan
Copy link
Member

@tsullivan tsullivan commented Aug 13, 2024

Summary

Needed for: elastic/kibana#184496

Context: EUI exports a class called Query which is responsible for parsing search terms into an AST. The parse() method of the class accepts a ParseOptions object, which is what has been changed in this PR. The Query class and ParseOptions objects come into play when consumers of EuiSearchBar wish to manage the query outside of the component, otherwise the EuiSearchBar component will control parsing the search terms into an AST internally.

What this does: This PR updates the "default syntax" parser, used to determine queries for the Search Bar component, to support a "recognizedFields" optional option.

Why: When used for searching structured content that is managed by Kibana (i.e., the Global Search Bar and Saved Objects Finders) the search bar should work more intuitively by using logic to infer what the user is trying to do. If the search bar is designed to support field clauses such as type:typeA or tag:tagB, it would be illogical to infer that remote-cluster:my-remote-index is a field clause.

QA

General checklist

  • Docs site QA - N/A, skipping due to advanced and slightly Kibana-specific usage
  • Code quality checklist
  • Release checklist
    • A changelog entry exists and is marked appropriately.

@tsullivan tsullivan force-pushed the eui/parse-query-update branch from 42b3539 to c5a8707 Compare August 13, 2024 22:18
@tsullivan tsullivan marked this pull request as ready for review August 19, 2024 23:42
@tsullivan tsullivan requested a review from a team as a code owner August 19, 2024 23:42
@tsullivan tsullivan requested a review from pugnascotia August 20, 2024 17:11
Copy link
Contributor

@cee-chen cee-chen left a comment

Choose a reason for hiding this comment

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

This looks great to me from an EUI point of view, thank you for the fantastic work Tim! I'll wait until end of week for @pugnascotia to chime in with any query/AST-specific feedback before merging.

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

fieldName "field name"
= identifier
= id:identifier &{ return !hasRecognizedFields || recognizedFields.includes(id); } { return id; }
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the ampersand doing here?

Copy link
Member Author

Choose a reason for hiding this comment

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

@pugnascotia

Unfortunately, PegJS documentation is no longer available directly, but it can be found here: http://web.archive.org/web/20240102234657/https://pegjs.org/documentation. Take a look under "Parsing Expression Types"

& expression
Try to match the expression. If the match succeeds, just return undefined and do not consume any input, otherwise consider the match failed.

In other words, the &{ return !hasRecognizedFields || recognizedFields.includes(id); } is a predicate check that determines whether the rule succeeds or fails. This check ensures that if hasRecognizedFields is truthy, then there is a restriction that the identifier must be included in the recognizedFields array to be treated as a field name. If the predicate passes, then the identifier is parsed as a field name. If the predicate does not pass, then the fieldName match is failed and the parser falls back to using the identifier rule.

By the way, I used Microsoft Copilot to explain this to me and help me with updating the parsing rules. If it wasn't for that, I am sure I would not have been able to figure it out.

Copy link
Contributor

Choose a reason for hiding this comment

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

I checked the docs as they appear in source control, to double-check:

https://github.com/pegjs/pegjs/blob/master/docs/grammar/parsing-expression-types.md#--predicate-

So this looks good.

@tsullivan tsullivan requested a review from pugnascotia August 21, 2024 20:47
Copy link
Contributor

@pugnascotia pugnascotia left a comment

Choose a reason for hiding this comment

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

LGTM

@tkajtoch tkajtoch merged commit 54ab23a into elastic:main Aug 22, 2024
5 checks passed
Ikuni17 pushed a commit to elastic/kibana that referenced this pull request Aug 31, 2024
`v95.7.0` ⏩ `v95.9.0`

> [!CAUTION]
> This PR contains the final set of Emotion conversions for all EuiForm
components.
> If your plugin contains any custom CSS/styling to **EuiFormRow,
EuiFormControlLayout, EuiCheckbox, EuiRadio, or EuiSwitch**, ⚠️ make
sure to QA your UI to ensure no visual regressions have occurred! ⚠️

---

## [`v95.9.0`](https://github.com/elastic/eui/releases/v95.9.0)

- Updated `EuiSearchBar`'s optional `box.schema` prop with a new
`recognizedFields` configuration. This allows specifying the phrases
that will be parsed as field clauses
([#7960](elastic/eui#7960))
- Updated `EuiIcon` with a new `tokenSemanticText` glyph
([#7971](elastic/eui#7971))
- Added support for TypeScript 5
([#7980](elastic/eui#7980))

**Bug fixes**

- Fixed `EuiSelectableTemplateSitewide` styles when used within a
dark-themed `EuiHeader`
([#7977](elastic/eui#7977))

## [`v95.8.0`](https://github.com/elastic/eui/releases/v95.8.0)

- Updated `EuiHeaderLinks`'s mobile menu to set a slight popover padding
by default ([#7961](elastic/eui#7961))
- This can be overridden via `popoverProps.panelPaddingSize` if needed.
- Updated `EuiHeaderLink` to default to a size of `s` (down from `m`)
([#7961](elastic/eui#7961))

**Accessibility**

- Updated the `aria-label` attribute for the `EuiFieldSearch` clear
button ([#7970](elastic/eui#7970))

**Bug fixes**

- Fixed a visual bug with `<EuiDualRange showInput="inputWithPopover"
/>` form controls ([#7957](elastic/eui#7957))

**Deprecations**

- Deprecated `EuiFormRow`'s `columnCompressedSwitch` display prop. Use
`columnCompressed` instead, which will automatically account for child
`EuiSwitch`es ([#7968](elastic/eui#7968))
- Deprecated `EuiFormRow`'s `rowCompressed` display prop. Use `row`
instead for vertical forms, or `centerCompressed` for inline forms
([#7968](elastic/eui#7968))
- (Styling) Updated `EuiFormRow`'s `hasEmptySpaceLabel` prop to no
longer attempt to automatically align its content to a vertical center.
Use the `display="center"` prop for that instead
([#7968](elastic/eui#7968))

**CSS-in-JS conversions**

- Converted `EuiFormControlLayout` to Emotion
([#7954](elastic/eui#7954))
- Removed `.euiFormControlLayout--*icons` classNames and
`--eui-form-control-layout-icons-padding` CSS var. Use
`--euiFormControlRightIconsCount` or `--euiFormControlLeftIconsCount`
instead
- Converted `EuiFormLayoutDelimited` to Emotion
([#7957](elastic/eui#7957))
- Fixed `cloneElementWithCss` throwing an error when used multiple times
without a `key` prop ([#7957](elastic/eui#7957))
- Updated `cloneElementWithCss` utility to support a third argument that
allows prepending vs. appending the cloned Emotion css className
([#7957](elastic/eui#7957))
- Removed `@euiFormControlLayoutClearIcon` Sass mixin
([#7959](elastic/eui#7959))
- Converted `EuiDescribedFormGroup` to Emotion
([#7964](elastic/eui#7964))
- Converted `EuiForm`, `EuiFormHelpText`, and `EuiFormErrorText` to
Emotion ([#7966](elastic/eui#7966))
- Converted `EuiFormLabel` and `EuiFormLegend` to Emotion; Removed
`@euiFormLabel` mixin
([#7967](elastic/eui#7967))
- Converted `EuiFormRow` to Emotion
([#7968](elastic/eui#7968))
- Converted `EuiCheckbox` to Emotion
([#7969](elastic/eui#7969))
- Converted `EuiRadio` to Emotion
([#7969](elastic/eui#7969))
- Converted `EuiSwitch` to Emotion
([#7969](elastic/eui#7969))
- Removed the following Sass variables:
([#7969](elastic/eui#7969))
  - `$euiFormCustomControlDisabledIconColor`
  - `$euiFormCustomControlBorderColor`
  - `$euiRadioSize`
  - `$euiCheckBoxSize`
  - `$euiCheckboxBorderRadius`
  - `$euiSwitchHeight` (and compressed/mini variants)
  - `$euiSwitchWidth` (and compressed/mini variants)
  - `$euiSwitchThumbSize` (and compressed/mini variants)
  - `$euiSwitchIconHeight`
  - `$euiSwitchOffColor`
- Removed the following Sass mixins:
([#7969](elastic/eui#7969))
  - `euiIconBackground`
  - `euiCustomControl`
  - `euiCustomControlSelected`
  - `euiCustomControlDisabled`
  - `euiCustomControlFocused`

---------

Co-authored-by: Marta Bondyra <4283304+mbondyra@users.noreply.github.com>
tsullivan added a commit to elastic/kibana that referenced this pull request Sep 19, 2024
…cify recognized fields (#190464)

This PR adds a new option to the `SchemaType` interface for parsing a
query from a search in EuiSearchBar. This new field controls how
EuiSearchBar text is parsed into a Query object. It enables better
accuracy in how search terms are parsed when they include a `:`
character.

## Release note
Fixed an issue when using search bars with a term that includes a colon
`:` character.

## Summary

Closes #184496
Depends on elastic/eui#7960

**GLOBAL SEARCH: BEFORE**

![akshfgkalsfh-before](https://github.com/user-attachments/assets/22377a3e-394e-43fb-83db-ae2477ce22d7)

**GLOBAL SEARCH: AFTER**

![akshfgkalsfh-after](https://github.com/user-attachments/assets/406d56eb-c946-493b-94f3-abb0380611f3)

**SAVED OBJECTS MANAGEMENT: BEFORE**

![okjoyofjiuh-before](https://github.com/user-attachments/assets/c1c56572-31aa-41df-b0c5-3eef421a06c5)

**SAVED OBJECTS MANAGEMENT: AFTER**

![okjoyofjiuh-after](https://github.com/user-attachments/assets/9e19bbcf-72e7-43d5-a9e7-3c5805632a38)

**SAVED OBJECTS FINDER: BEFORE**

![lfdgnhklfd-before](https://github.com/user-attachments/assets/b826987d-8af6-4c20-93b0-0d0bb76d9501)

**SAVED OBJECTS FINDER: AFTER**

![lfdgnhklfd-after](https://github.com/user-attachments/assets/e8e007b9-91f7-4209-bfd5-ce43a2cbc894)

## Checklist
 - [x] Ensure that filtering using `type:` and `tags:` still works
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Sep 19, 2024
…cify recognized fields (elastic#190464)

This PR adds a new option to the `SchemaType` interface for parsing a
query from a search in EuiSearchBar. This new field controls how
EuiSearchBar text is parsed into a Query object. It enables better
accuracy in how search terms are parsed when they include a `:`
character.

## Release note
Fixed an issue when using search bars with a term that includes a colon
`:` character.

## Summary

Closes elastic#184496
Depends on elastic/eui#7960

**GLOBAL SEARCH: BEFORE**

![akshfgkalsfh-before](https://github.com/user-attachments/assets/22377a3e-394e-43fb-83db-ae2477ce22d7)

**GLOBAL SEARCH: AFTER**

![akshfgkalsfh-after](https://github.com/user-attachments/assets/406d56eb-c946-493b-94f3-abb0380611f3)

**SAVED OBJECTS MANAGEMENT: BEFORE**

![okjoyofjiuh-before](https://github.com/user-attachments/assets/c1c56572-31aa-41df-b0c5-3eef421a06c5)

**SAVED OBJECTS MANAGEMENT: AFTER**

![okjoyofjiuh-after](https://github.com/user-attachments/assets/9e19bbcf-72e7-43d5-a9e7-3c5805632a38)

**SAVED OBJECTS FINDER: BEFORE**

![lfdgnhklfd-before](https://github.com/user-attachments/assets/b826987d-8af6-4c20-93b0-0d0bb76d9501)

**SAVED OBJECTS FINDER: AFTER**

![lfdgnhklfd-after](https://github.com/user-attachments/assets/e8e007b9-91f7-4209-bfd5-ce43a2cbc894)

## Checklist
 - [x] Ensure that filtering using `type:` and `tags:` still works

(cherry picked from commit 460ca2a)
kibanamachine referenced this pull request in elastic/kibana Sep 19, 2024
…to specify recognized fields (#190464) (#193448)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Global Search, Saved Objects Management] Use new parse option to
specify recognized fields
(#190464)](#190464)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Tim
Sullivan","email":"tsullivan@users.noreply.github.com"},"sourceCommit":{"committedDate":"2024-09-19T16:02:45Z","message":"[Global
Search, Saved Objects Management] Use new parse option to specify
recognized fields (#190464)\n\nThis PR adds a new option to the
`SchemaType` interface for parsing a\r\nquery from a search in
EuiSearchBar. This new field controls how\r\nEuiSearchBar text is parsed
into a Query object. It enables better\r\naccuracy in how search terms
are parsed when they include a `:`\r\ncharacter.\r\n\r\n## Release
note\r\nFixed an issue when using search bars with a term that includes
a colon\r\n`:` character.\r\n\r\n## Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/184496\r\nDepends on
https://github.com/elastic/eui/pull/7960\r\n\r\n**GLOBAL SEARCH:
BEFORE**\r\n\r\n![akshfgkalsfh-before](https://github.com/user-attachments/assets/22377a3e-394e-43fb-83db-ae2477ce22d7)\r\n\r\n**GLOBAL
SEARCH:
AFTER**\r\n\r\n![akshfgkalsfh-after](https://github.com/user-attachments/assets/406d56eb-c946-493b-94f3-abb0380611f3)\r\n\r\n**SAVED
OBJECTS MANAGEMENT:
BEFORE**\r\n\r\n![okjoyofjiuh-before](https://github.com/user-attachments/assets/c1c56572-31aa-41df-b0c5-3eef421a06c5)\r\n\r\n**SAVED
OBJECTS MANAGEMENT:
AFTER**\r\n\r\n![okjoyofjiuh-after](https://github.com/user-attachments/assets/9e19bbcf-72e7-43d5-a9e7-3c5805632a38)\r\n\r\n**SAVED
OBJECTS FINDER:
BEFORE**\r\n\r\n![lfdgnhklfd-before](https://github.com/user-attachments/assets/b826987d-8af6-4c20-93b0-0d0bb76d9501)\r\n\r\n**SAVED
OBJECTS FINDER:
AFTER**\r\n\r\n![lfdgnhklfd-after](https://github.com/user-attachments/assets/e8e007b9-91f7-4209-bfd5-ce43a2cbc894)\r\n\r\n##
Checklist\r\n - [x] Ensure that filtering using `type:` and `tags:`
still
works","sha":"460ca2a83f0fd14b9d8c78c6b695e742c3f25930","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","v9.0.0","Team:SharedUX","v8.16.0","backport:version"],"title":"[Global
Search, Saved Objects Management] Use new parse option to specify
recognized
fields","number":190464,"url":"https://github.com/elastic/kibana/pull/190464","mergeCommit":{"message":"[Global
Search, Saved Objects Management] Use new parse option to specify
recognized fields (#190464)\n\nThis PR adds a new option to the
`SchemaType` interface for parsing a\r\nquery from a search in
EuiSearchBar. This new field controls how\r\nEuiSearchBar text is parsed
into a Query object. It enables better\r\naccuracy in how search terms
are parsed when they include a `:`\r\ncharacter.\r\n\r\n## Release
note\r\nFixed an issue when using search bars with a term that includes
a colon\r\n`:` character.\r\n\r\n## Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/184496\r\nDepends on
https://github.com/elastic/eui/pull/7960\r\n\r\n**GLOBAL SEARCH:
BEFORE**\r\n\r\n![akshfgkalsfh-before](https://github.com/user-attachments/assets/22377a3e-394e-43fb-83db-ae2477ce22d7)\r\n\r\n**GLOBAL
SEARCH:
AFTER**\r\n\r\n![akshfgkalsfh-after](https://github.com/user-attachments/assets/406d56eb-c946-493b-94f3-abb0380611f3)\r\n\r\n**SAVED
OBJECTS MANAGEMENT:
BEFORE**\r\n\r\n![okjoyofjiuh-before](https://github.com/user-attachments/assets/c1c56572-31aa-41df-b0c5-3eef421a06c5)\r\n\r\n**SAVED
OBJECTS MANAGEMENT:
AFTER**\r\n\r\n![okjoyofjiuh-after](https://github.com/user-attachments/assets/9e19bbcf-72e7-43d5-a9e7-3c5805632a38)\r\n\r\n**SAVED
OBJECTS FINDER:
BEFORE**\r\n\r\n![lfdgnhklfd-before](https://github.com/user-attachments/assets/b826987d-8af6-4c20-93b0-0d0bb76d9501)\r\n\r\n**SAVED
OBJECTS FINDER:
AFTER**\r\n\r\n![lfdgnhklfd-after](https://github.com/user-attachments/assets/e8e007b9-91f7-4209-bfd5-ce43a2cbc894)\r\n\r\n##
Checklist\r\n - [x] Ensure that filtering using `type:` and `tags:`
still
works","sha":"460ca2a83f0fd14b9d8c78c6b695e742c3f25930"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/190464","number":190464,"mergeCommit":{"message":"[Global
Search, Saved Objects Management] Use new parse option to specify
recognized fields (#190464)\n\nThis PR adds a new option to the
`SchemaType` interface for parsing a\r\nquery from a search in
EuiSearchBar. This new field controls how\r\nEuiSearchBar text is parsed
into a Query object. It enables better\r\naccuracy in how search terms
are parsed when they include a `:`\r\ncharacter.\r\n\r\n## Release
note\r\nFixed an issue when using search bars with a term that includes
a colon\r\n`:` character.\r\n\r\n## Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/184496\r\nDepends on
https://github.com/elastic/eui/pull/7960\r\n\r\n**GLOBAL SEARCH:
BEFORE**\r\n\r\n![akshfgkalsfh-before](https://github.com/user-attachments/assets/22377a3e-394e-43fb-83db-ae2477ce22d7)\r\n\r\n**GLOBAL
SEARCH:
AFTER**\r\n\r\n![akshfgkalsfh-after](https://github.com/user-attachments/assets/406d56eb-c946-493b-94f3-abb0380611f3)\r\n\r\n**SAVED
OBJECTS MANAGEMENT:
BEFORE**\r\n\r\n![okjoyofjiuh-before](https://github.com/user-attachments/assets/c1c56572-31aa-41df-b0c5-3eef421a06c5)\r\n\r\n**SAVED
OBJECTS MANAGEMENT:
AFTER**\r\n\r\n![okjoyofjiuh-after](https://github.com/user-attachments/assets/9e19bbcf-72e7-43d5-a9e7-3c5805632a38)\r\n\r\n**SAVED
OBJECTS FINDER:
BEFORE**\r\n\r\n![lfdgnhklfd-before](https://github.com/user-attachments/assets/b826987d-8af6-4c20-93b0-0d0bb76d9501)\r\n\r\n**SAVED
OBJECTS FINDER:
AFTER**\r\n\r\n![lfdgnhklfd-after](https://github.com/user-attachments/assets/e8e007b9-91f7-4209-bfd5-ce43a2cbc894)\r\n\r\n##
Checklist\r\n - [x] Ensure that filtering using `type:` and `tags:`
still
works","sha":"460ca2a83f0fd14b9d8c78c6b695e742c3f25930"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Tim Sullivan <tsullivan@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants