-
Notifications
You must be signed in to change notification settings - Fork 26
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(shadows): background theming issue #737
Conversation
20274e1
to
8e547eb
Compare
src/Uno.Toolkit.Skia.WinUI/Controls/Shadows/ShadowContainer.Paint.cs
Outdated
Show resolved
Hide resolved
src/Uno.Toolkit.Skia.WinUI/Controls/Shadows/ShadowContainer.Paint.cs
Outdated
Show resolved
Hide resolved
src/Uno.Toolkit.Skia.WinUI/Controls/Shadows/ShadowContainer.Paint.cs
Outdated
Show resolved
Hide resolved
Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-737.eastus2.azurestaticapps.net |
@agneszitte edit: this also fixes the other one too. |
sanity-check done: winui, skia, wasm, ios, droid |
25f1d46
to
a003cea
Compare
Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-737.eastus2.azurestaticapps.net |
a003cea
to
e72a29d
Compare
Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-737.eastus2.azurestaticapps.net |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm seeing some strange behaviors if I add an extra shadow using the ShadowContainerSamplePage here (tested on Skia so far):
It seems to also break on main
but in a different way:
After adding new shadow:
Not sure if it's really related to these changes but something is different
@kazo0 There might be re-layout issue with the SC on skia, that was previously not much noticeable due to background/border breaking on changes on skia. I think this just uncovers it. |
@Mergifyio backport legacy/3x |
✅ Backports have been created
|
GitHub Issue (If applicable): closes: #707
PR Type
What kind of change does this PR introduce?
What is the current behavior?
ShadowContainer currently strips its child's background and use it as the background for the image-brush. This is done because the inner shadows need to be drawn on top of the content background. This approach has a few problems where it doesnt subscribes to updates to the background property, nor theme updates from the
{ThemeResource}
brush.What is the new behavior?
ShadowContainer now creates two layers of shadows, one for background shadows and one for foreground shadows, that sandwiches the content.
PR Checklist
Please check if your PR fulfills the following requirements:
Other information