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: skia and IOS shadows not updating properly #756

Merged
merged 1 commit into from
Aug 31, 2023

Conversation

Marc-Antoine-Soucy
Copy link
Contributor

@Marc-Antoine-Soucy Marc-Antoine-Soucy commented Aug 24, 2023

GitHub Issue (If applicable): #748

PR Type

What kind of change does this PR introduce?

  • Bugfix

What is the current behavior?

Right now, on skia, and IOS you add a shadow to a shadow container and increase the size of the shadow content, the shadows appear too small,
image

What is the new behavior?

They stay the right size and visible
shadowskia

PR Checklist

Please check if your PR fulfills the following requirements:

@github-actions
Copy link

github-actions bot commented Aug 24, 2023

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-756.eastus2.azurestaticapps.net

@Marc-Antoine-Soucy Marc-Antoine-Soucy marked this pull request as ready for review August 29, 2023 21:20
@github-actions
Copy link

github-actions bot commented Aug 29, 2023

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-756.eastus2.azurestaticapps.net

@Xiaoy312 Xiaoy312 self-requested a review August 29, 2023 21:38
@agneszitte agneszitte linked an issue Aug 29, 2023 that may be closed by this pull request
@Marc-Antoine-Soucy
Copy link
Contributor Author

Maybe ActualWidth/Height from both contentAsFE and _shadowHost should be in the key, to make sure we don't get another bug like that again?

@Marc-Antoine-Soucy Marc-Antoine-Soucy changed the title fix: skia shadows not updating properly fix: skia and IOS shadows not updating properly Aug 30, 2023
@github-actions
Copy link

github-actions bot commented Aug 30, 2023

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-756.eastus2.azurestaticapps.net

@agneszitte
Copy link
Contributor

Should we add some tests for this @jeromelaban? Or on a following PR?

@jeromelaban
Copy link
Member

Should we add some tests for this @jeromelaban? Or on a following PR?

Definitely. This looks like a behavior that can break easily, I'd also say that using formatting for this type of keying is probably not the most performant way to do so.

@Marc-Antoine-Soucy let's merge this PR if it's approved by the rest of the team, but make sure to add a create a new PR right after that will add regression testing. I've removed the automatic closing of the issue so we can keep track of this.

@agneszitte
Copy link
Contributor

Should we add some tests for this @jeromelaban? Or on a following PR?

Definitely. This looks like a behavior that can break easily, I'd also say that using formatting for this type of keying is probably not the most performant way to do so.

@Marc-Antoine-Soucy let's merge this PR if it's approved by the rest of the team, but make sure to add a create a new PR right after that will add regression testing. I've removed the automatic closing of the issue so we can keep track of this.

@jeromelaban, @Marc-Antoine-Soucy I entered an issue for tracking
#791

@agneszitte agneszitte enabled auto-merge (squash) August 31, 2023 01:51
@agneszitte
Copy link
Contributor

@Mergifyio backport legacy/3x release/stable/3.1

@mergify
Copy link
Contributor

mergify bot commented Aug 31, 2023

backport legacy/3x release/stable/3.1

✅ Backports have been created

@agneszitte agneszitte merged commit 2c9c84a into main Aug 31, 2023
24 checks passed
@agneszitte agneszitte deleted the dev/maso/shadowcontainerskiafix branch August 31, 2023 02:00
mergify bot pushed a commit that referenced this pull request Aug 31, 2023
mergify bot pushed a commit that referenced this pull request Aug 31, 2023
agneszitte pushed a commit that referenced this pull request Aug 31, 2023
Co-authored-by: Marc-Antoine-Soucy <90481654+Marc-Antoine-Soucy@users.noreply.github.com>
agneszitte pushed a commit that referenced this pull request Aug 31, 2023
Co-authored-by: Marc-Antoine-Soucy <90481654+Marc-Antoine-Soucy@users.noreply.github.com>
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.

[Skia / iOS] ShadowContainer re-layout issue
4 participants