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(tooltip, popover): Support transparent backgrounds #6803

Closed
wants to merge 5 commits into from

Conversation

driskull
Copy link
Member

@driskull driskull commented Apr 17, 2023

Related Issue: #6798

Summary

  • Uses css clip-path instead of transform: rotate to create the caret arrow tail.
  • Removes CSS where we are setting the BG on two separate elements.
  • Screener tests

@github-actions github-actions bot added the bug Bug reports for broken functionality. Issues should include a reproduction of the bug. label Apr 17, 2023
@driskull driskull added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Apr 17, 2023
@driskull driskull removed the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Apr 17, 2023
@driskull driskull added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Apr 17, 2023
@driskull driskull marked this pull request as ready for review April 18, 2023 20:44
@driskull driskull requested a review from a team as a code owner April 18, 2023 20:44
Copy link
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

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

🎉

@driskull
Copy link
Member Author

@jcfranco @ashetland Let me know if you'd like to proceed with this or not.

@ashetland
Copy link

Losing the border on the arrow is a deal breaker for me on this one. Without the border, the arrow practically disappears and thus reads like a bug. I think supporting transparency, even if minimal, is valid though. What would it mean to support it? Are we talking breaking changes / major rework?

@driskull
Copy link
Member Author

@jcfranco you'll have to make the call on what is more important here.

If we have an arrow behind an arrow that will defeat the purpose of transparency as well due to compounding background colors.

@driskull driskull assigned jcfranco and unassigned driskull Apr 19, 2023
@driskull driskull requested a review from jcfranco April 20, 2023 22:38
@driskull
Copy link
Member Author

Going to close this as it's not possible to use just clip-path and have a border with it. I even tried putting a rotated box div inside and trying to clip it but it still didn't render correctly.

Going forward, we may want to introduce an internal component (possibly functional component) to handle rendering a triangle SVG and styling it with a stroke/fill.

See: floating-ui/floating-ui#2195

CSS just isn't good at doing this even in 2023.

@driskull driskull closed this Apr 24, 2023
@driskull driskull deleted the dris0000/popover-tooltip-tail branch April 24, 2023 21:44
jcfranco pushed a commit that referenced this pull request Apr 26, 2023
…ng (#6086)

**Related Issue:** #6081
[#6803](#6083)

## Summary
As suggested by @alisonailea [in this
comment](#6075 (comment)),
extracts some common keyboard navigation focus helpers into a shared
utility.

Along the way discovered a few things that are fixed alongside the
refactors:

Accordion:
- Cleanup: The keyboard expected navigation changed at some point, which
I confirmed with @geospatialem, but there was a lot of related stale
code that is removed.

Dropdown:
- Fix: Previously, tab / shift tab would skip elements resulting in
navigating to every other item.
- Fix: Previously, there was inconsistent close behavior when using tab
/ shift tab at the first / last item.
- Fix: Previously, when `accordion-item` had a populated `href`
property, there was a "double focus" issue.
- Cleanup: Uses the new utility to handle keyboard navigation / focus


Stepper / Tabs:
- Cleanup: Uses the new utility to handle keyboard navigation / focus


All existing tests are passing, but we may want some extra passes of
manual testing, especially in the dropdown case, since there were so
many weird bugs previously.
driskull added a commit that referenced this pull request Apr 26, 2023
**Related Issue:** #6798

## Summary

- Creates a `FloatingArrow` functional component to generate an SVG for
the arrow.
- Uses `FloatingArrow` instead of `transform: rotate` to create the
caret arrow tail.
- Introduces optional state property `floatingLayout` on
`FloatingUIComponent` class for sizing the arrow properly.
- Removes CSS where we are setting the BG on two separate elements.
- Sets `Action` used in the popover to be transparent.
- Removes unnecessary styles
- Screener tests
- See: floating-ui/floating-ui#2195
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug reports for broken functionality. Issues should include a reproduction of the bug. pr ready for visual snapshots Adding this label will run visual snapshot testing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants