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

Hide backdrop for popover #624

Merged
merged 5 commits into from
May 24, 2024
Merged

Conversation

Kitenite
Copy link
Contributor

Certain sites might color their popover backdrop, overriding to keep it visible.
https://ng-matero.github.io/ng-matero

Screenshot 2024-05-22 at 4 00 46 PM
Screenshot 2024-05-22 at 4 00 58 PM

@argyleink
Copy link
Member

oh yes, good call, visbug popovers will inherit any backdrop styles. we def don't want that!

thanks for the site where we can test 👍🏻

curious your thoughts here:
your PR just blanket removes sets any backdrop to transparent, but what about including in each of the web component shadow roots a style that prevents inheriting an opaque backdrop? the web components already have a bunch of defensive styles, and now they're popovers, they could include popover defensive styles.

I verified the bug is fixed with the following styles. What i like about this is that it's not changing the host page, it's changing visbug.

vis-bug::backdrop, 
visbug-handles::backdrop, 
visbug-hover::backdrop, 
visbug-distance::backdrop {
  background: none;
}

if we like this route better, i believe we just add the following to each elements shadow root that is a popover.

::backdrop {
  background: none;
}

the PR would touch more files (like this one) but i think the solution is less leaky or side effecty? thoughts? do you think it would surprise someone if they launched visbug and all their backdrops disappeared?

@Kitenite
Copy link
Contributor Author

Sounds good let me push an update!

@Kitenite
Copy link
Contributor Author

I think this should be all the popover. Seems to work in test for me.

@argyleink
Copy link
Member

since the CI no longer auto deploys the extension, and by the looks of this PR test run more is broken.. I'll need to do it Monday next week or so..

but thanks for this! looks great 🎉

@argyleink argyleink merged commit 5fc0488 into GoogleChromeLabs:main May 24, 2024
1 of 2 checks passed
@Kitenite Kitenite deleted the backdrop branch May 24, 2024 20:33
@argyleink
Copy link
Member

just uploaded a new build, sorry it took so long! I have excused, but listing them wouldn't make anything better.

@Kitenite
Copy link
Contributor Author

No worries, thanks for the maintenance!

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.

2 participants