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

[Unified search] Fixes the css priority order for the filter pills #156639

Merged
merged 3 commits into from
May 5, 2023

Conversation

stratoula
Copy link
Contributor

@stratoula stratoula commented May 4, 2023

Summary

Closes #156446

There were cases were the css defined by our custom class were overwritten by the eui badge classes. This doesn't happen always, I could replicate it only when the legacy input controls are present.

image

@stratoula stratoula added release_note:fix Feature:Unified search Unified search related tasks backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) v8.8.0 v8.9.0 labels May 4, 2023
@stratoula stratoula marked this pull request as ready for review May 4, 2023 07:31
@stratoula stratoula requested a review from a team as a code owner May 4, 2023 07:31
@stratoula stratoula added the Team:Visualizations Visualization editors, elastic-charts and infrastructure label May 4, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-visualizations @elastic/kibana-visualizations-external (Team:Visualizations)

Copy link
Contributor

@dej611 dej611 left a comment

Choose a reason for hiding this comment

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

Code review only 👍

Copy link
Contributor

@mdefazio mdefazio left a comment

Choose a reason for hiding this comment

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

LGTM. Had one comment but not a blocker as it wasn't caused by this PR.

@@ -18,15 +18,15 @@
}
}

.globalFilterItem-isDisabled {
.euiBadge.globalFilterItem-isDisabled {
color: $euiColorDarkShade;
background-color: transparentize($euiColorLightShade, .5);
text-decoration: line-through;
font-weight: $euiFontWeightRegular;
font-style: italic;
Copy link
Contributor

Choose a reason for hiding this comment

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

I couldn't see where this was actually showing the italic font (before or after this pr). Is it worth removing with this PR since it's touching these classes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanx @mdefazio, done 484f11d

@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
unifiedSearch 268.4KB 268.5KB +54.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
securitySolution 398 401 +3
total +5

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
securitySolution 478 481 +3
total +5

History

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

@stratoula stratoula merged commit eda3c85 into elastic:main May 5, 2023
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request May 5, 2023
…lastic#156639)

## Summary

Closes elastic#156446

There were cases were the css defined by our custom class were
overwritten by the eui badge classes. This doesn't happen always, I
could replicate it only when the legacy input controls are present.

<img width="2504" alt="image"
src="https://user-images.githubusercontent.com/17003240/236125401-d81925de-09e6-4580-9be0-3e1315c40d41.png">

(cherry picked from commit eda3c85)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.8

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 May 5, 2023
…lls (#156639) (#156839)

# Backport

This will backport the following commits from `main` to `8.8`:
- [[Unified search] Fixes the css priority order for the filter pills
(#156639)](#156639)

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

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

<!--BACKPORT [{"author":{"name":"Stratoula
Kalafateli","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-05-05T12:05:40Z","message":"[Unified
search] Fixes the css priority order for the filter pills
(#156639)\n\n## Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/156446\r\n\r\nThere were cases
were the css defined by our custom class were\r\noverwritten by the eui
badge classes. This doesn't happen always, I\r\ncould replicate it only
when the legacy input controls are present.\r\n\r\n<img width=\"2504\"
alt=\"image\"\r\nsrc=\"https://user-images.githubusercontent.com/17003240/236125401-d81925de-09e6-4580-9be0-3e1315c40d41.png\">","sha":"eda3c85bb4f26eb05c210412d37971d3b07ac009","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Visualizations","Feature:Unified
search","backport:prev-minor","v8.8.0","v8.9.0"],"number":156639,"url":"https://github.com/elastic/kibana/pull/156639","mergeCommit":{"message":"[Unified
search] Fixes the css priority order for the filter pills
(#156639)\n\n## Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/156446\r\n\r\nThere were cases
were the css defined by our custom class were\r\noverwritten by the eui
badge classes. This doesn't happen always, I\r\ncould replicate it only
when the legacy input controls are present.\r\n\r\n<img width=\"2504\"
alt=\"image\"\r\nsrc=\"https://user-images.githubusercontent.com/17003240/236125401-d81925de-09e6-4580-9be0-3e1315c40d41.png\">","sha":"eda3c85bb4f26eb05c210412d37971d3b07ac009"}},"sourceBranch":"main","suggestedTargetBranches":["8.8"],"targetPullRequestStates":[{"branch":"8.8","label":"v8.8.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/156639","number":156639,"mergeCommit":{"message":"[Unified
search] Fixes the css priority order for the filter pills
(#156639)\n\n## Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/156446\r\n\r\nThere were cases
were the css defined by our custom class were\r\noverwritten by the eui
badge classes. This doesn't happen always, I\r\ncould replicate it only
when the legacy input controls are present.\r\n\r\n<img width=\"2504\"
alt=\"image\"\r\nsrc=\"https://user-images.githubusercontent.com/17003240/236125401-d81925de-09e6-4580-9be0-3e1315c40d41.png\">","sha":"eda3c85bb4f26eb05c210412d37971d3b07ac009"}}]}]
BACKPORT-->

Co-authored-by: Stratoula Kalafateli <[email protected]>
cee-chen added a commit that referenced this pull request Oct 2, 2024
## Summary

This PR removes the `euiFormControlDefaultShadow` mixin from Kibana
usage, which is shortly set to be deprecated/removed from EUI.

The usages of this mixin primarily wanted the `border` styling of the
mixin and not its background effects, so I've opted to simplify the CSS
greatly to simply use `border` CSS instead of attempting to mess around
with `box-shadow` (which wasn't really benefiting the final visual
appearance of the affected use cases).

I also incidentally removed some extra CSS specificity added in #156639
(no longer necessary as of #161592) which was causing exclusive borders
to not be the correct color.

| Before | After |
|--------|--------|
| <img width="696" alt=""
src="https://github.com/user-attachments/assets/00478e77-08e8-490d-90fa-84abd2d3ba99">
| <img width="704" alt=""
src="https://github.com/user-attachments/assets/46ef0a5f-5fb0-4d47-82ba-40ed7eb2ff89">
|

### 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)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 2, 2024
## Summary

This PR removes the `euiFormControlDefaultShadow` mixin from Kibana
usage, which is shortly set to be deprecated/removed from EUI.

The usages of this mixin primarily wanted the `border` styling of the
mixin and not its background effects, so I've opted to simplify the CSS
greatly to simply use `border` CSS instead of attempting to mess around
with `box-shadow` (which wasn't really benefiting the final visual
appearance of the affected use cases).

I also incidentally removed some extra CSS specificity added in elastic#156639
(no longer necessary as of elastic#161592) which was causing exclusive borders
to not be the correct color.

| Before | After |
|--------|--------|
| <img width="696" alt=""
src="https://github.com/user-attachments/assets/00478e77-08e8-490d-90fa-84abd2d3ba99">
| <img width="704" alt=""
src="https://github.com/user-attachments/assets/46ef0a5f-5fb0-4d47-82ba-40ed7eb2ff89">
|

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

(cherry picked from commit 7edb90e)
kibanamachine added a commit that referenced this pull request Oct 2, 2024
…es (#194653) (#194730)

# Backport

This will backport the following commits from `main` to `8.x`:
- [Remove Sass &#x60;@euiFormControlDefaultShadow&#x60; mixin usages
(#194653)](#194653)

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

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

<!--BACKPORT [{"author":{"name":"Cee
Chen","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-02T15:37:19Z","message":"Remove
Sass `@euiFormControlDefaultShadow` mixin usages (#194653)\n\n##
Summary\r\n\r\nThis PR removes the `euiFormControlDefaultShadow` mixin
from Kibana\r\nusage, which is shortly set to be deprecated/removed from
EUI.\r\n\r\nThe usages of this mixin primarily wanted the `border`
styling of the\r\nmixin and not its background effects, so I've opted to
simplify the CSS\r\ngreatly to simply use `border` CSS instead of
attempting to mess around\r\nwith `box-shadow` (which wasn't really
benefiting the final visual\r\nappearance of the affected use
cases).\r\n\r\nI also incidentally removed some extra CSS specificity
added in #156639\r\n(no longer necessary as of #161592) which was
causing exclusive borders\r\nto not be the correct color.\r\n\r\n|
Before | After |\r\n|--------|--------|\r\n| <img width=\"696\"
alt=\"\"\r\nsrc=\"https://github.com/user-attachments/assets/00478e77-08e8-490d-90fa-84abd2d3ba99\">\r\n|
<img width=\"704\"
alt=\"\"\r\nsrc=\"https://github.com/user-attachments/assets/46ef0a5f-5fb0-4d47-82ba-40ed7eb2ff89\">\r\n|\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)","sha":"7edb90e8bdc82a338ab8e1c4626bd9bfa69ee3f4","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","EUI","v9.0.0","v8.16.0","backport:version"],"title":"Remove
Sass `@euiFormControlDefaultShadow` mixin
usages","number":194653,"url":"https://github.com/elastic/kibana/pull/194653","mergeCommit":{"message":"Remove
Sass `@euiFormControlDefaultShadow` mixin usages (#194653)\n\n##
Summary\r\n\r\nThis PR removes the `euiFormControlDefaultShadow` mixin
from Kibana\r\nusage, which is shortly set to be deprecated/removed from
EUI.\r\n\r\nThe usages of this mixin primarily wanted the `border`
styling of the\r\nmixin and not its background effects, so I've opted to
simplify the CSS\r\ngreatly to simply use `border` CSS instead of
attempting to mess around\r\nwith `box-shadow` (which wasn't really
benefiting the final visual\r\nappearance of the affected use
cases).\r\n\r\nI also incidentally removed some extra CSS specificity
added in #156639\r\n(no longer necessary as of #161592) which was
causing exclusive borders\r\nto not be the correct color.\r\n\r\n|
Before | After |\r\n|--------|--------|\r\n| <img width=\"696\"
alt=\"\"\r\nsrc=\"https://github.com/user-attachments/assets/00478e77-08e8-490d-90fa-84abd2d3ba99\">\r\n|
<img width=\"704\"
alt=\"\"\r\nsrc=\"https://github.com/user-attachments/assets/46ef0a5f-5fb0-4d47-82ba-40ed7eb2ff89\">\r\n|\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)","sha":"7edb90e8bdc82a338ab8e1c4626bd9bfa69ee3f4"}},"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/194653","number":194653,"mergeCommit":{"message":"Remove
Sass `@euiFormControlDefaultShadow` mixin usages (#194653)\n\n##
Summary\r\n\r\nThis PR removes the `euiFormControlDefaultShadow` mixin
from Kibana\r\nusage, which is shortly set to be deprecated/removed from
EUI.\r\n\r\nThe usages of this mixin primarily wanted the `border`
styling of the\r\nmixin and not its background effects, so I've opted to
simplify the CSS\r\ngreatly to simply use `border` CSS instead of
attempting to mess around\r\nwith `box-shadow` (which wasn't really
benefiting the final visual\r\nappearance of the affected use
cases).\r\n\r\nI also incidentally removed some extra CSS specificity
added in #156639\r\n(no longer necessary as of #161592) which was
causing exclusive borders\r\nto not be the correct color.\r\n\r\n|
Before | After |\r\n|--------|--------|\r\n| <img width=\"696\"
alt=\"\"\r\nsrc=\"https://github.com/user-attachments/assets/00478e77-08e8-490d-90fa-84abd2d3ba99\">\r\n|
<img width=\"704\"
alt=\"\"\r\nsrc=\"https://github.com/user-attachments/assets/46ef0a5f-5fb0-4d47-82ba-40ed7eb2ff89\">\r\n|\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)","sha":"7edb90e8bdc82a338ab8e1c4626bd9bfa69ee3f4"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Cee Chen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) Feature:Unified search Unified search related tasks release_note:fix Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.8.0 v8.9.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Unified search] [Legacy controls] Problem with the UI of the created filters
6 participants