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

Add iconType and disableLanguageSwitcher options to QueryStringInput #83700

Merged
merged 8 commits into from
Nov 23, 2020

Conversation

jen-huang
Copy link
Contributor

@jen-huang jen-huang commented Nov 18, 2020

Summary

This PR adds new props to QueryStringInput component:

  • iconType: string to show an icon in front of the input field
  • disableLanguageSwitcher: boolean to hide the language switcher at the end of the input field

Both new props are optional. Default behavior of the component is not changed (no icon, and visible language switcher).

I had to make some border styling adjustments to maintain the border when there is multiline input but not show a right-side border when language switcher is disabled. The previous styling had a translateX(-1) on the input, so the right-side border was always visible. With these changes, that transform was removed, but the border styling differs minimally from the original implementation in #70140.

The use case for these props came up in #83356, where we implemented a feature that requires a query bar with suggestions that is locked to KQL and design spec included a search icon be present.

HQ screenshots to compare styling: with icon and language switcher present to illustrate input boundaries

image

image

image

Low quality gifs showing behavior

With icon and language switcher disabled:

Nov-18-2020 11-21-04

With only icon:

Nov-18-2020 11-21-47

With only language switcher disabled:

Nov-18-2020 11-22-10

Default behavior:

Nov-18-2020 11-22-30

Checklist

@jen-huang jen-huang added Feature:Query Bar Querying and query bar features enhancement New value added to drive a business result v8.0.0 Team:AppArch release_note:skip Skip the PR/issue when compiling release notes v7.11.0 labels Nov 18, 2020
@jen-huang jen-huang self-assigned this Nov 18, 2020
@jen-huang jen-huang marked this pull request as ready for review November 18, 2020 19:40
@jen-huang jen-huang requested review from a team as code owners November 18, 2020 19:40
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

Copy link
Contributor

@snide snide left a comment

Choose a reason for hiding this comment

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

Did a quick code review. Thanks for adding to the docs and the gifs. Reasoning looks sound. Small comments.

@jen-huang
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@lizozom lizozom left a comment

Choose a reason for hiding this comment

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

Looks great!
I just wonder if we shouldn't make the icon visible everywhere, rather than have it configurable.

@lizozom
Copy link
Contributor

lizozom commented Nov 22, 2020

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
data 245.5KB 245.9KB +486.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
data 974.0KB 974.9KB +932.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@snide
Copy link
Contributor

snide commented Nov 23, 2020

Looks great!
I just wonder if we shouldn't make the icon visible everywhere, rather than have it configurable.

I'd keep it off by default. There's already a lot of icons around in the default and the placeholder gives context.

@jen-huang
Copy link
Contributor Author

Thanks @snide and @lizozom for the reviews! :)

@jen-huang jen-huang merged commit 81f4dc9 into elastic:master Nov 23, 2020
@jen-huang jen-huang deleted the data/query-input branch November 23, 2020 15:06
jen-huang added a commit that referenced this pull request Nov 23, 2020
…nput` (#83700) (#84097)

* Add iconType and disableLanguageSwitcher options to QueryStringInput

* Remove unnecessary span, add tests

* Update docs

* Adjust suggestions offset

* Add comment

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <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
enhancement New value added to drive a business result Feature:Query Bar Querying and query bar features release_note:skip Skip the PR/issue when compiling release notes v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants