Skip to content

experiment: add popover base styles and visual tests #9638

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

Merged
merged 4 commits into from
Jul 7, 2025

Conversation

web-padawan
Copy link
Member

Description

Depends on #9631

Type of change

  • Experiment

@web-padawan web-padawan requested review from jouni and DiegoCardoso July 4, 2025 07:57
@DiegoCardoso
Copy link
Contributor

Focusing the popover with keyboard shows an outline that seems a bit broken and not aligned with the focus ring in the base theme. I checked in Lumo, and there's no focus ring when the popover is focused by keyboard. Should we remove the outline, change it, or keep it this way?

image

@web-padawan web-padawan force-pushed the experiment/base-popover branch from 799daf9 to fe0e654 Compare July 4, 2025 11:21
@web-padawan
Copy link
Member Author

@DiegoCardoso Thanks, added missing styles and updated arrow to adapt to the outline. Also added visual tests.

@jouni
Copy link
Member

jouni commented Jul 4, 2025

I have some improvements in the works, I'll continue with them on Monday.

Copy link

sonarqubecloud bot commented Jul 7, 2025

@jouni
Copy link
Member

jouni commented Jul 7, 2025

I ended up partially refactoring how the arrow is rendered. It’s now clipped to not overlap the content, so we don't need positioning and z-index.

Here’s an illustration of how the clip-path works, to make it easier to grok the code:
image

The shadow ends up getting rotated as well, so it’s not going to be 100% accurate. But it’s mostly to cover the case when the overlay part's border is implemented with a semi-transparent 1px outer box-shadow. We’d need direction-specific box-shadows to make it perfect, but that’s up to theme implementations to consider if it’s worth it.

@web-padawan web-padawan merged commit 391f9aa into main Jul 7, 2025
10 checks passed
@web-padawan web-padawan deleted the experiment/base-popover branch July 7, 2025 15:59
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 25.0.0-alpha3.

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

Successfully merging this pull request may close these issues.

4 participants