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(iOS): extraLight blur not working #2338

Merged
merged 9 commits into from
Oct 18, 2024
Merged

Conversation

maksg
Copy link
Contributor

@maksg maksg commented Sep 10, 2024

Description

Blur style UIBlurEffectStyleExtraLight equals 0 so using it in if (config.blurEffect) makes it that the code for applying backgroundEffect doesn't execute.

With this fix the navigation bar can no longer be fully transparent because blur effect is always set as extraLight by default. So it's up for a discussion if it should stay that way or if we should make this value optional.

Fixes #2320.

Test code and steps to reproduce

Added Test2320 to tests.

Checklist

  • Included code example that can be used to test this change
  • Ensured that CI passes

Copy link
Member

@kkafar kkafar left a comment

Choose a reason for hiding this comment

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

Good catch, but we rather don't want to go this way.
Let's add a option unspecified, but only for internal API usage (do not include this option in public API), and when blurEffect has undefined / nullish value, in bottom-level Screen component (just in the wrapper for NativeScreenComponent) let's make use of this Unspecified / Undefined enum variant and pass it to native side. Then in headerConfig check for variant value.

Does that sound right?

@maksg
Copy link
Contributor Author

maksg commented Sep 16, 2024

@kkafar I added custom enum value UIBlurEffectStyleUndefined so it looks pretty clean.
Let me know if this approach is good enough or I should change it.

Copy link
Member

@kkafar kkafar left a comment

Choose a reason for hiding this comment

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

Thank you @maksg!

I've changed undefined for none and intorduced proxy type RNSBlurEffectStyle so that we can properly differentiate when the blur effect is not set.

@kkafar
Copy link
Member

kkafar commented Oct 17, 2024

I'll merge this once CI passes.

@kkafar kkafar self-assigned this Oct 18, 2024
@kkafar kkafar merged commit fcb1693 into main Oct 18, 2024
8 checks passed
@kkafar kkafar deleted the @maksg/fix-ios-extra-light-blur branch October 18, 2024 09:10
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.

[NativeStack] headerBlurEffect: "extraLight" has no effect
2 participants