Skip to content

Conversation

@nickofthyme
Copy link
Contributor

@nickofthyme nickofthyme commented Dec 5, 2024

Summary

Adds a z-index of -1 to popover arrow to prevent stacking on top of popover content.

This issue was noticed in the Lens color mapping UI where we set panelPaddingSize="none", see code here. We are not applying any special styles that I can tell but the arrow shows on top of the content, you can see this better when a background color is applied, see below...

Zight Recording 2024-12-05 at 02 27 02 PM

I was not able to reproduce this in eui docs but another improvement with this change that I did see is the focus state of the popover.

Before After
Chrome Popover - Elastic UI Framework 2024-12-05 at 2 55 55 PM Popover - Elastic UI Framework 2024-12-05 at 2 56 12 PM
Firefox firefox-before firefox-after

Notice how the arrow slightly occludes the border.

Maybe there is a better solution or issues with this one, open to alternatives.

QA

Remove or strikethrough items that do not apply to your PR.

General checklist

  • Browser QA
    • Checked in both light and dark modes
    • Checked in mobile
    • Checked in Chrome, Safari, Edge, and Firefox
    • Checked for accessibility including keyboard-only and screenreader modes
  • Release checklist
    • A changelog entry exists and is marked appropriately.
    • If applicable, added the breaking change issue label (and filled out the breaking change checklist)
  • Designer checklist
    • If applicable, file an issue to update EUI's Figma library with any corresponding UI changes. (This is an internal repo, if you are external to Elastic, ask a maintainer to submit this request)

@nickofthyme nickofthyme requested a review from a team as a code owner December 5, 2024 20:52
@nickofthyme nickofthyme requested a review from markov00 December 5, 2024 20:54
@kibanamachine
Copy link

Preview staging links for this PR:

@elasticmachine
Copy link
Collaborator

💔 Build Failed

Failed CI Steps

@cee-chen
Copy link
Contributor

cee-chen commented Dec 6, 2024

Once the current high contrast mode PR merges, we'll be using clip-path to fix that issue, which should be picked up by Borealis once it rebases against the high contrast work - see #8174 (comment)

@cee-chen cee-chen closed this Dec 6, 2024
@nickofthyme
Copy link
Contributor Author

nickofthyme commented Dec 6, 2024

Ok nice, are your changes already in kibana main?

If so, can you confirm this is fixed in kibana? I am developing in kibana with the latest borealis theme and still see this issue.

image

@cee-chen
Copy link
Contributor

cee-chen commented Dec 6, 2024

They are not in Kibana main and it will be a while before it reaches Kibana main - is the fix urgent? If so, I'd ping @mgadewoll to make this temporary fix in the Borealis feature branch.

@nickofthyme
Copy link
Contributor Author

is the fix urgent

No that should be fine. Do you know if it will be in 9.0?

@nickofthyme nickofthyme deleted the fix-popover-arrow branch December 6, 2024 17:58
@cee-chen
Copy link
Contributor

cee-chen commented Dec 6, 2024

Yes, it should be in by then 🤞 If high contrast work isn't in Borealis, at the very minimum Lene can copy the arrow CSS (sans high contrast logic) from it.

@mgadewoll
Copy link
Contributor

ℹ️ I added this PR to introduce the popover arrow changes to Borealis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants