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 transparent default modal background #23985

Merged
merged 3 commits into from
Aug 5, 2024
Merged

Conversation

PureWeen
Copy link
Member

@PureWeen PureWeen commented Aug 2, 2024

Description of Change

By default if the user hasn't set a background on the page, we'll set a background on the page so they can't see the content underneath the modal

Issues Fixed

Fixes #23973

@PureWeen PureWeen marked this pull request as ready for review August 2, 2024 18:16
@PureWeen PureWeen requested a review from a team as a code owner August 2, 2024 18:16
@PureWeen PureWeen requested review from jsuarezruiz and tj-devel709 and removed request for a team August 2, 2024 18:16
}


if (e.IsOneOf(Page.BackgroundColorProperty, Page.BackgroundProperty))
Copy link
Member

Choose a reason for hiding this comment

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

I think BackgroundColor will always fire Background, so this is allocating an array twice and then firing twice for each time the Color is set.

Copy link
Member Author

Choose a reason for hiding this comment

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

the Background Mapper will fire but the Background INPC won't fire

Here's a quick test
image

@PureWeen
Copy link
Member Author

PureWeen commented Aug 5, 2024

  • Failing test is unrelated, API 23 is currently having issues with xharness

@PureWeen PureWeen merged commit 4c287d0 into release/9.0.1xx-preview7 Aug 5, 2024
75 of 76 checks passed
@PureWeen PureWeen deleted the fix_23973 branch August 5, 2024 17:31
@github-actions github-actions bot locked and limited conversation to collaborators Sep 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants