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

fix for search due to eui@29.0.0 #77962

Merged
merged 3 commits into from
Sep 21, 2020
Merged

fix for search due to eui@29.0.0 #77962

merged 3 commits into from
Sep 21, 2020

Conversation

snide
Copy link
Contributor

@snide snide commented Sep 18, 2020

Summary

This is an implementation fix due to #77802

The new search template has some new requirements around breakpoints and mobile buttons. These props were absent in the upgrade PR and forced the search to have issues with blur states.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@snide snide requested a review from cchaos September 18, 2020 22:35
@snide
Copy link
Contributor Author

snide commented Sep 18, 2020

@cchaos we probably need to think up a way to handle the mobile stuff a little cleaner in Kibana, but this addresses the main issues. cc @myasonik for awareness.

@snide snide added the REASSIGN from Team:Core UI Deprecated label for old Core UI team label Sep 18, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core-ui (Team:Core UI)

@snide snide added v7.10.0 release_note:skip Skip the PR/issue when compiling release notes labels Sep 18, 2020
Copy link
Contributor

@ryankeairns ryankeairns left a comment

Choose a reason for hiding this comment

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

Reviewed tabbing, works as expected. Thanks Dave!

@snide
Copy link
Contributor Author

snide commented Sep 21, 2020

@elasticmachine merge upstream

@shahzad31
Copy link
Contributor

it has other few quirky behaviours, for example sometimes clearing the searchbox doesn't reset the result list

image

@shahzad31
Copy link
Contributor

Also if you use purely keyboard to focus and search, then it doesn't auto select the first entry in the list.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

page load bundle size

id value diff baseline
globalSearchBar 27.9KB +458.0B 27.5KB

History

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

@snide snide merged commit ed46b1b into elastic:master Sep 21, 2020
@snide snide deleted the fix/search branch September 21, 2020 19:02
@snide
Copy link
Contributor Author

snide commented Sep 21, 2020

@shahzad31 I'll leave that feedback to the core ui team. This was a quick regression fix that I'd like to get to master.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 77962 or prevent reminders by adding the backport:skip label.

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Sep 23, 2020
@myasonik
Copy link
Contributor

@snide Did you backport this?

@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Sep 24, 2020
snide added a commit that referenced this pull request Sep 24, 2020
Fixes a regression in global search due to the EUI upgrade. The search now operates better on mobile screens.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
REASSIGN from Team:Core UI Deprecated label for old Core UI team release_note:skip Skip the PR/issue when compiling release notes v7.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants