[EuiSearchBar] Use "must" semantics for simple_query_string terms in DSL gen
#6717
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6717/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6717/ |
s-nel
changed the title
AND as default_operator for simple_query_stringsimple_query_string terms in DSL gen
cee-chen
changed the title
simple_query_string terms in DSL gensimple_query_string terms in DSL gen
|
So first - apologies, I'm a super noob at DSL and unfortunately most of the people who originally wrote @tobio / @PhaedrusTheGreek - y'all have contributed some awesome bugfixes to |
|
Hey @tobio / @PhaedrusTheGreek - one more super quick ping on this PR! If I don't hear back, I'll go ahead and merge it after this Tuesday's release (giving us another week before it hits Kibana). Most of the history I'm seeing in EuiSearchBar is around term queries and not simple query strings, so my (extremely uneducated) guess is that it's probably relatively safe. |
|
I had a quick look at this and it seems sensible. I won't be able to take a proper review before Wednesday though, I'm happy for this to be merged, but haven't done a proper review. |
|
No worries, thanks so much for responding! If @s-nel has no objections or isn't in a hurry, we can wait until this Wednesday for you to take a more thorough look! |
|
I'm in no rush |
|
Woo hoo! I really appreciate the help from both of y'all. You rock! |
jbudz
pushed a commit
to elastic/kibana
that referenced
this pull request
May 15, 2023
## Summary `eui@77.2.2` ⏩ `eui@79.0.1` 🦴 The primary changes in this upgrade are around the deprecated `EuiLoadingContent` being removed in favor of `EuiSkeletonText`. - Most instances have been a [direct swap of usage](327626a), but [some replacements were a bit more opinionated](e6ceb36) as I saw them as potential to take advantage of `EuiSkeletonText`'s syntactical sugar and screen reader announcements for when state switches to loaded. --- ## [`79.0.1`](https://github.com/elastic/eui/tree/v79.0.1) **Bug fixes** - Fixed broken push `EuiFlyout` behavior ([#6764](elastic/eui#6764)) ## [`79.0.0`](https://github.com/elastic/eui/tree/v79.0.0) - Updated all `EuiSkeleton` components with new props that allow for more control over screen reader live announcements: `announceLoadingStatus`, `announceLoadedStatus`, and `ariaLiveProps` ([#6752](elastic/eui#6752)) - Improved keyboard accessibility in `EuiPageHeader` by ensuring the right side menu items come into focus from left to right. ([#6753](elastic/eui#6753)) **Breaking changes** - Removed deprecated `EuiLoadingContent`. Use the `EuiSkeleton` components instead. ([#6754](elastic/eui#6754)) ## [`78.0.0`](https://github.com/elastic/eui/tree/v78.0.0) - Improved the contrast ratio of `EuiCheckbox`, `EuiRadio`, and `EuiSwitch` in their unchecked states to meet WCAG AA guidelines. ([#6729](elastic/eui#6729)) - Added React Testing Library `*ByTestSubject` custom commands to `within()`. RTL utilities can be imported from `@elastic/eui/lib/test/rtl`. ([#6737](elastic/eui#6737)) - Updated `EuiAvatar` to support a new letter `casing` prop that allow customizing text capitalization ([#6739](elastic/eui#6739)) - Updated `EuiFocusTrap` to support the `gapMode` prop configuration (now defaults to `padding`) ([#6744](elastic/eui#6744)) **Bug fixes** - Fixed inconsistency in `EuiSearchBar`'s AND/OR semantics between DSL and query string generation ([#6717](elastic/eui#6717)) - Fixed `EuiFieldNumber`'s native browser validity detection causing extra unnecessary rerenders ([#6741](elastic/eui#6741)) - Fixed the `scrollLock` property on `EuiFocusTrap` (and other components using `EuiFocusTrap`, such as `EuiFlyout` and `EuiModal`) to no longer block scrolling on nested portalled content, such as combobox dropdowns ([#6744](elastic/eui#6744)) **Breaking changes** - `EuiAvatar`s with the default `user` type will now default to capitalizing all initials in uppercase ([#6739](elastic/eui#6739)) --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
jasonrhodes
pushed a commit
to elastic/kibana
that referenced
this pull request
May 17, 2023
## Summary `eui@77.2.2` ⏩ `eui@79.0.1` 🦴 The primary changes in this upgrade are around the deprecated `EuiLoadingContent` being removed in favor of `EuiSkeletonText`. - Most instances have been a [direct swap of usage](327626a), but [some replacements were a bit more opinionated](e6ceb36) as I saw them as potential to take advantage of `EuiSkeletonText`'s syntactical sugar and screen reader announcements for when state switches to loaded. --- ## [`79.0.1`](https://github.com/elastic/eui/tree/v79.0.1) **Bug fixes** - Fixed broken push `EuiFlyout` behavior ([#6764](elastic/eui#6764)) ## [`79.0.0`](https://github.com/elastic/eui/tree/v79.0.0) - Updated all `EuiSkeleton` components with new props that allow for more control over screen reader live announcements: `announceLoadingStatus`, `announceLoadedStatus`, and `ariaLiveProps` ([#6752](elastic/eui#6752)) - Improved keyboard accessibility in `EuiPageHeader` by ensuring the right side menu items come into focus from left to right. ([#6753](elastic/eui#6753)) **Breaking changes** - Removed deprecated `EuiLoadingContent`. Use the `EuiSkeleton` components instead. ([#6754](elastic/eui#6754)) ## [`78.0.0`](https://github.com/elastic/eui/tree/v78.0.0) - Improved the contrast ratio of `EuiCheckbox`, `EuiRadio`, and `EuiSwitch` in their unchecked states to meet WCAG AA guidelines. ([#6729](elastic/eui#6729)) - Added React Testing Library `*ByTestSubject` custom commands to `within()`. RTL utilities can be imported from `@elastic/eui/lib/test/rtl`. ([#6737](elastic/eui#6737)) - Updated `EuiAvatar` to support a new letter `casing` prop that allow customizing text capitalization ([#6739](elastic/eui#6739)) - Updated `EuiFocusTrap` to support the `gapMode` prop configuration (now defaults to `padding`) ([#6744](elastic/eui#6744)) **Bug fixes** - Fixed inconsistency in `EuiSearchBar`'s AND/OR semantics between DSL and query string generation ([#6717](elastic/eui#6717)) - Fixed `EuiFieldNumber`'s native browser validity detection causing extra unnecessary rerenders ([#6741](elastic/eui#6741)) - Fixed the `scrollLock` property on `EuiFocusTrap` (and other components using `EuiFocusTrap`, such as `EuiFlyout` and `EuiModal`) to no longer block scrolling on nested portalled content, such as combobox dropdowns ([#6744](elastic/eui#6744)) **Breaking changes** - `EuiAvatar`s with the default `user` type will now default to capitalizing all initials in uppercase ([#6739](elastic/eui#6739)) --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
This was referenced Aug 1, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Currently there is a discrepancy between query string generation and DSL generation: query string generation treats all terms as
ANDed together by default because it uses must (+). DSL alsoANDs clauses together by default except in the case of terms. In this case the terms areOR'd due to Elasticsearch's default behavior with thesimple_query_stringquery.This PR makes DSL and query string generation consistent by adding
+prefix to terms so that any terms that are in the samesimple_query_stringquery are AND'd together.Before
After
QA
Remove or strikethrough items that do not apply to your PR.
General checklist
- [ ] Checked in both light and dark modes- [ ] Checked in mobile- [ ] Checked in Chrome, Safari, Edge, and Firefox- [ ] Checked Code Sandbox works for any docs examples- [ ] Props have proper autodocs (using@defaultif default values are missing) and playground toggles- [ ] Checked for accessibility including keyboard-only and screenreader modes- [ ] Updated the Figma library counterpart