Skip to content

[ObsUX] Remove background for dropdown button, visual refresh bug#221366

Merged
MiriamAparicio merged 5 commits intoelastic:mainfrom
MiriamAparicio:fix-button-background-visual-refresh
May 26, 2025
Merged

[ObsUX] Remove background for dropdown button, visual refresh bug#221366
MiriamAparicio merged 5 commits intoelastic:mainfrom
MiriamAparicio:fix-button-background-visual-refresh

Conversation

@MiriamAparicio
Copy link
Copy Markdown
Contributor

@MiriamAparicio MiriamAparicio commented May 23, 2025

Summary

Related to elastic/kibana#220620

Following a review of the upcoming button visual refresh planned by the EUI team for early June, we identified an issue with the background styling on dropdown buttons within the Infrastructure Inventory view.

image

Changes Implemented

  • Removed the background from the dropdown button label.
  • Added a right border to improve visual clarity.

These changes are safe to merge into main ahead of the EUI release, as they do not impact the current UI behavior.

@MiriamAparicio MiriamAparicio requested a review from a team May 23, 2025 12:24
@MiriamAparicio MiriamAparicio added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting Team:obs-ux-infra_services - DEPRECATED DEPRECATED - Use Team:obs-presentation. EUI Visual Refresh v9.1.0 labels May 23, 2025
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services)

Copy link
Copy Markdown
Contributor

@iblancof iblancof left a comment

Choose a reason for hiding this comment

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

I tried it locally, and it looks like we’re still seeing a background on :focus and :active.
I’m not sure if this PR was meant to address that as well.

Screen.Recording.2025-05-26.at.10.42.01.mov

<EuiFlexItem grow={false}>
<EuiButtonEmpty
aria-label={i18n.translate('xpack.infra.dropdownButton.button.ariaLabel', {
defaultMessage: label,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this intended? The label message does not seem contextual enough in this case.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You're right, I will add something else to the message, like "Metric options", "Group by options" etc

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cool thanks!

I realize now that the translation key might not be very specific, so it could apply to any dropdown button within infra. I’m not sure if we have any specific guidelines around translation keys.

@MiriamAparicio
Copy link
Copy Markdown
Contributor Author

I tried it locally, and it looks like we’re still seeing a background on :focus and :active.
I’m not sure if this PR was meant to address that as well.

Thanks @iblancof for testing, I will recheck it, but the background was fixed against the EUI PR, to not show when hover and to add the border as they suggested in a slack thread
Note that

These changes are safe to merge into main ahead of the EUI release, as they do not impact the current UI behavior.

@MiriamAparicio MiriamAparicio force-pushed the fix-button-background-visual-refresh branch from 7609e20 to dc42b41 Compare May 26, 2025 09:23
@MiriamAparicio
Copy link
Copy Markdown
Contributor Author

Hi @iblancof

This is the final result agaist EUI PR, when they release the changes this is how it's going to look, no background on ::active or ::focus

Screen.Recording.2025-05-26.at.10.12.14.mov

@elasticmachine
Copy link
Copy Markdown
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

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
infra 1.1MB 1.1MB +284.0B

History

@MiriamAparicio MiriamAparicio merged commit 970f009 into elastic:main May 26, 2025
10 checks passed
@MiriamAparicio MiriamAparicio deleted the fix-button-background-visual-refresh branch May 26, 2025 15:07
akowalska622 pushed a commit to akowalska622/kibana that referenced this pull request May 29, 2025
…astic#221366)

## Summary
Related to
[elastic#220620](elastic#220620)

Following a review of the upcoming button visual refresh planned by the
EUI team for early June, we identified an issue with the background
styling on dropdown buttons within the Infrastructure Inventory view.

<img width="496" alt="image"
src="https://github.com/user-attachments/assets/5e293034-fd44-4d0e-824b-a66863c0ae8a"
/>

### Changes Implemented

- Removed the background from the dropdown button label.
- Added a right border to improve visual clarity.

These changes are safe to merge into `main` ahead of the EUI release, as
they do not impact the current UI behavior.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting EUI Visual Refresh release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-infra_services - DEPRECATED DEPRECATED - Use Team:obs-presentation. v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants