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.

Sign up for GitHub

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

Jump to bottom

[Discover] Fix Fields flyout and Create Field flyouts on mobile #145650

Merged
merged 4 commits into from
Nov 19, 2022
Merged

[Discover] Fix Fields flyout and Create Field flyouts on mobile #145650

merged 4 commits into from
Nov 19, 2022

Conversation

cee-chen
Copy link
Member

@cee-chen cee-chen commented Nov 17, 2022 •
edited by kibanamachine
Loading

Summary

@jughosta flagged several responsive issues in #144845 (review). Two of these were existing issues in Kibana main and need to be backported to 8.6, hence the separate PR.

I recommend turning off whitespace changes when viewing the diff, as much of the wrapped/unwrapped code remained the same except for indentation.

Create Field flyout:

Before:

After:

Fields flyout (mobile only):

Before:

After:

Checklist

@cee-chen
Fix non-scrolling flyout
386e271 
- `.euiFlyoutBody` no longer contains any meaningful CSS after EuiFlyout's conversion to Emotion, causing the bug

- All this section really needs is `height: 100%` and `overflow-y: scroll`, which DiscoverSidebar's CSS already contains. Removing the wrapper solves the issue

+ tweaked padding
@cee-chen
Fix create field flyout in mobile/responsive views
586b976 
- `maxWidth={1180}` appears to enforce that width on all resolutions. Switching to a conditional maxWidth and removing the custom inline styles on smaller screens resolves the issue
@cee-chen cee-chen added bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes v8.6.0 v8.7.0 labels Nov 17, 2022
@cee-chen cee-chen requested review from a team as code owners November 17, 2022 23:23
@cee-chen cee-chen mentioned this pull request Nov 17, 2022
Merged
@cee-chen
Copy link
Member Author

@elasticmachine merge upstream

@kertal kertal requested a review from mattkime November 18, 2022 06:39
jughosta
jughosta approved these changes Nov 18, 2022
View reviewed changes
Copy link
Contributor

@jughosta jughosta left a comment

Choose a reason for hiding this comment

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

Thanks for fixing it, @constancecchen ! 🙌

@kibana-ci
Copy link
Collaborator

💚 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
dataViewFieldEditor 154.4KB 154.5KB +112.0B
discover 414.4KB 414.3KB -77.0B
total +35.0B
Unknown metric groups

ESLint disabled in files

id before after diff
osquery 1 2 +1

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
fleet 59 65 +6
osquery 108 113 +5
securitySolution 442 448 +6
total +19

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
fleet 67 73 +6
osquery 109 115 +6
securitySolution 519 525 +6
total +20

History

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

mattkime
mattkime approved these changes Nov 18, 2022
View reviewed changes
Copy link
Contributor

@mattkime mattkime left a comment

Choose a reason for hiding this comment

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

changes lgtm, thanks for the fix!

@cee-chen cee-chen merged commit 10c6649 into elastic:main Nov 19, 2022
@cee-chen cee-chen deleted the discover-flyouts branch November 19, 2022 06:44
@kibanamachine kibanamachine mentioned this pull request Nov 19, 2022
Merged
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 19, 2022
[Discover] Fix Fields flyout and Create Field flyouts on mobile (elas…
470e926 
…tic#145650)

## Summary

@jughosta flagged several responsive issues in
elastic#144845 (review).
Two of these were existing issues in Kibana main and need to be
backported to 8.6, hence the separate PR.

I recommend turning off whitespace changes when viewing the diff, as
much of the wrapped/unwrapped code remained the same except for
indentation.

### Create Field flyout:

#### Before:
<img width="760" alt=""
src="https://user-images.githubusercontent.com/549407/202580476-610e3f54-eabb-4859-a49d-19d0608be787.png">

#### After:
<img width="779" alt=""
src="https://user-images.githubusercontent.com/549407/202580282-886435e7-00cc-4569-943d-fff471b3af7b.png">

### Fields flyout (mobile only):

#### Before:
<img width="754" alt=""
src="https://user-images.githubusercontent.com/549407/202580465-917ef121-d0eb-4188-bcf7-2b08edb5dd49.png">

#### After:
<img width="761" alt=""
src="https://user-images.githubusercontent.com/549407/202580275-820b9420-a8da-458e-bbb4-5f1cfb57ba99.png">

### Checklist

- [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)

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Julia Rechkunova <julia.rechkunova@elastic.co>
(cherry picked from commit 10c6649)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.6

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Nov 19, 2022
@kibanamachine
[8.6] [Discover] Fix Fields flyout and Create Field flyouts on mobile (…
9e75cc2 
…#145650) (#145789)

# Backport

This will backport the following commits from `main` to `8.6`:
- [[Discover] Fix Fields flyout and Create Field flyouts on mobile
(#145650)](#145650)

<!--- Backport version: 8.9.7 -->

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

<!--BACKPORT
[{"author":{"name":"Constance","email":"constancecchen@users.noreply.github.com"},"sourceCommit":{"committedDate":"2022-11-19T06:44:28Z","message":"[Discover]
Fix Fields flyout and Create Field flyouts on mobile (#145650)\n\n##
Summary\r\n\r\n@jughosta flagged several responsive issues
in\r\nhttps://github.com//pull/144845#pullrequestreview-1183919709.\r\nTwo
of these were existing issues in Kibana main and need to
be\r\nbackported to 8.6, hence the separate PR.\r\n\r\nI recommend
turning off whitespace changes when viewing the diff, as\r\nmuch of the
wrapped/unwrapped code remained the same except
for\r\nindentation.\r\n\r\n### Create Field flyout:\r\n\r\n####
Before:\r\n<img width=\"760\"
alt=\"\"\r\nsrc=\"https://user-images.githubusercontent.com/549407/202580476-610e3f54-eabb-4859-a49d-19d0608be787.png\">\r\n\r\n####
After:\r\n<img width=\"779\"
alt=\"\"\r\nsrc=\"https://user-images.githubusercontent.com/549407/202580282-886435e7-00cc-4569-943d-fff471b3af7b.png\">\r\n\r\n###
Fields flyout (mobile only):\r\n\r\n#### Before:\r\n<img width=\"754\"
alt=\"\"\r\nsrc=\"https://user-images.githubusercontent.com/549407/202580465-917ef121-d0eb-4188-bcf7-2b08edb5dd49.png\">\r\n\r\n####
After:\r\n<img width=\"761\"
alt=\"\"\r\nsrc=\"https://user-images.githubusercontent.com/549407/202580275-820b9420-a8da-458e-bbb4-5f1cfb57ba99.png\">\r\n\r\n###
Checklist\r\n\r\n- [x] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[x] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\nCo-authored-by:
Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Julia Rechkunova
<julia.rechkunova@elastic.co>","sha":"10c6649f4b28184fc07d41f081fac7e56c447a50","branchLabelMapping":{"^v8.7.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","v8.6.0","v8.7.0"],"number":145650,"url":"#145650
Fix Fields flyout and Create Field flyouts on mobile (#145650)\n\n##
Summary\r\n\r\n@jughosta flagged several responsive issues
in\r\nhttps://github.com//pull/144845#pullrequestreview-1183919709.\r\nTwo
of these were existing issues in Kibana main and need to
be\r\nbackported to 8.6, hence the separate PR.\r\n\r\nI recommend
turning off whitespace changes when viewing the diff, as\r\nmuch of the
wrapped/unwrapped code remained the same except
for\r\nindentation.\r\n\r\n### Create Field flyout:\r\n\r\n####
Before:\r\n<img width=\"760\"
alt=\"\"\r\nsrc=\"https://user-images.githubusercontent.com/549407/202580476-610e3f54-eabb-4859-a49d-19d0608be787.png\">\r\n\r\n####
After:\r\n<img width=\"779\"
alt=\"\"\r\nsrc=\"https://user-images.githubusercontent.com/549407/202580282-886435e7-00cc-4569-943d-fff471b3af7b.png\">\r\n\r\n###
Fields flyout (mobile only):\r\n\r\n#### Before:\r\n<img width=\"754\"
alt=\"\"\r\nsrc=\"https://user-images.githubusercontent.com/549407/202580465-917ef121-d0eb-4188-bcf7-2b08edb5dd49.png\">\r\n\r\n####
After:\r\n<img width=\"761\"
alt=\"\"\r\nsrc=\"https://user-images.githubusercontent.com/549407/202580275-820b9420-a8da-458e-bbb4-5f1cfb57ba99.png\">\r\n\r\n###
Checklist\r\n\r\n- [x] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[x] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\nCo-authored-by:
Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Julia Rechkunova
<julia.rechkunova@elastic.co>","sha":"10c6649f4b28184fc07d41f081fac7e56c447a50"}},"sourceBranch":"main","suggestedTargetBranches":["8.6"],"targetPullRequestStates":[{"branch":"8.6","label":"v8.6.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.7.0","labelRegex":"^v8.7.0$","isSourceBranch":true,"state":"MERGED","url":"#145650
Fix Fields flyout and Create Field flyouts on mobile (#145650)\n\n##
Summary\r\n\r\n@jughosta flagged several responsive issues
in\r\nhttps://github.com//pull/144845#pullrequestreview-1183919709.\r\nTwo
of these were existing issues in Kibana main and need to
be\r\nbackported to 8.6, hence the separate PR.\r\n\r\nI recommend
turning off whitespace changes when viewing the diff, as\r\nmuch of the
wrapped/unwrapped code remained the same except
for\r\nindentation.\r\n\r\n### Create Field flyout:\r\n\r\n####
Before:\r\n<img width=\"760\"
alt=\"\"\r\nsrc=\"https://user-images.githubusercontent.com/549407/202580476-610e3f54-eabb-4859-a49d-19d0608be787.png\">\r\n\r\n####
After:\r\n<img width=\"779\"
alt=\"\"\r\nsrc=\"https://user-images.githubusercontent.com/549407/202580282-886435e7-00cc-4569-943d-fff471b3af7b.png\">\r\n\r\n###
Fields flyout (mobile only):\r\n\r\n#### Before:\r\n<img width=\"754\"
alt=\"\"\r\nsrc=\"https://user-images.githubusercontent.com/549407/202580465-917ef121-d0eb-4188-bcf7-2b08edb5dd49.png\">\r\n\r\n####
After:\r\n<img width=\"761\"
alt=\"\"\r\nsrc=\"https://user-images.githubusercontent.com/549407/202580275-820b9420-a8da-458e-bbb4-5f1cfb57ba99.png\">\r\n\r\n###
Checklist\r\n\r\n- [x] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[x] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\nCo-authored-by:
Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Julia Rechkunova
<julia.rechkunova@elastic.co>","sha":"10c6649f4b28184fc07d41f081fac7e56c447a50"}}]}]
BACKPORT-->

Co-authored-by: Constance <constancecchen@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattkime mattkime mattkime approved these changes

@jughosta jughosta jughosta approved these changes

@MichaelMarcialis MichaelMarcialis MichaelMarcialis approved these changes

Assignees
No one assigned
Labels
bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes v8.6.0 v8.7.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@cee-chen @kibana-ci @kibanamachine @mattkime @jughosta @MichaelMarcialis