fix(android): stop losing view state on detach/re-attach + correctness (audit PR 1/6)#120
Merged
Merged
Conversation
…correctness Addresses the PR 1 batch from the July 2026 audit. BlurSwitch (critical): cleanup() ran from onDetachedFromWindow and nulled both the internal check-change listener and the onValueChange callback, which are only wired in the constructor and createViewInstance. react-native-screens detaches inactive screens and FlatList's removeClippedSubviews detaches off-screen cells, so after navigating away and back the switch stopped reporting toggles. cleanup() now runs only from the manager's onDropViewInstance (true teardown); the detach override is removed. BlurView: the same detach/re-attach pattern intermittently dropped the Screen-scoped blur root under react-navigation. The root swap + blur init now defer to a posted runnable (mirroring ProgressiveBlurView) so the parent chain and the QmBlurView library's own decor registration settle before the swap. This also gives the previously-dead initRunnable field a real purpose. Other correctness fixes: - Honor an explicit per-corner radius of 0 (test >= 0 against the -1 sentinel). - Refresh the glass effect when an invalid tint colour is caught. - Progressive blur: derive the effective radius from the raw amount in one place called from both setBlurAmount and setDirection, so the center-direction scale-down no longer depends on prop arrival order. - Progressive blur: clamp the center gradient startEdge to [0.01, 0.3] so the LinearGradient stops stay monotonic (above 0.3 they crossed over). - Remove the unused hasExplicitBackground field. Ship consumer-rules.pro keeping com.qmdeve.blurview classes/fields so a consumer app's R8 does not strip the fields the root swap reflects on and silently fall back to whole-screen blur (issue #89). Verified on an Android emulator (Pixel 6 Pro, API arm64): baseline toggle, toggle after tab navigation, and state persistence across a second navigation all behave correctly; no exceptions in logcat.
📝 WalkthroughWalkthroughAndroid blur behavior now includes published consumer R8 rules, deferred blur initialization, revised cleanup ownership, explicit zero-radius overrides, and centralized progressive blur radius calculation. ChangesAndroid blur updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
android/src/main/java/com/sbaiahmed1/reactnativeblur/ReactNativeBlurView.kt (1)
405-410: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse
topLeftinstead ofbaseRadiusfor uniform corner clipping.When all four specific corners are explicitly overridden to the same value (e.g., setting all corners to
0to remove the default rounding),isUniformevaluates totrue. PassingbaseRadiushere ignores the explicit override and incorrectly clips the view using the genericborderRadius.🐛 Proposed fix
if (isUniform) { outlineProvider = object : ViewOutlineProvider() { override fun getOutline(view: View, outline: Outline?) { - outline?.setRoundRect(0, 0, view.width, view.height, baseRadius) + outline?.setRoundRect(0, 0, view.width, view.height, topLeft) } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/src/main/java/com/sbaiahmed1/reactnativeblur/ReactNativeBlurView.kt` around lines 405 - 410, Update the uniform-corner outline provider in the isUniform branch to pass topLeft to setRoundRect instead of baseRadius, preserving explicit equal corner overrides such as zero.android/src/main/java/com/sbaiahmed1/reactnativeblur/ReactNativeProgressiveBlurView.kt (2)
358-367: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCorrect the gradient coordinates for horizontal directions.
The visual mappings for
leftToRightandrightToLeftare flipped and handle the offsets incorrectly, causing the blur to render on the wrong edge. For instance,leftToRight(blur on the left, clear on the right) currently assignspoint0(TRANSPARENT/clear) to the left side andpoint1(WHITE/blur) to the right side.Adjust the coordinates to correctly match the vertical gradient logic.
🐛 Proposed fix for directional mapping
"leftToRight" -> { + // Blur at left, clear at right + // point0 (TRANSPARENT/clear) at right, point1 (WHITE/blur) at left adjusted by offset val offsetPixels = width * currentStartOffset - floatArrayOf(offsetPixels, 0f, width.toFloat(), 0f) + floatArrayOf(width.toFloat(), 0f, offsetPixels, 0f) } "rightToLeft" -> { + // Blur at right, clear at left + // point0 (TRANSPARENT/clear) at left, point1 (WHITE/blur) at right adjusted by offset val offsetPixels = width * currentStartOffset - floatArrayOf(width.toFloat(), 0f, offsetPixels, 0f) + floatArrayOf(0f, 0f, width - offsetPixels, 0f) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/src/main/java/com/sbaiahmed1/reactnativeblur/ReactNativeProgressiveBlurView.kt` around lines 358 - 367, Correct the horizontal coordinate arrays in the direction mapping around the leftToRight and rightToLeft branches: assign the gradient’s transparent and blur endpoints to the same respective edges as the vertical logic, and apply currentStartOffset to the appropriate endpoint so each direction renders blur on the intended edge. Leave the vertical fallback unchanged.
78-88: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFix the blur mapping formula's denominator.
The formula incorrectly uses
MAX_BLUR_RADIUSas the denominator instead ofMAX_BLUR_AMOUNT. While this coincidentally evaluates to the same result right now because both constants happen to be100f, this logic will break entirely ifMAX_BLUR_RADIUSis corrected to25f(as documented in the comment above it).🐛 Proposed fix
private fun mapBlurAmountToRadius(amount: Float): Float { if (amount.isNaN() || amount.isInfinite()) { logWarning("Invalid blur amount: $amount, using default") return DEFAULT_BLUR_RADIUS } val clampedAmount = amount.coerceIn(MIN_BLUR_AMOUNT, MAX_BLUR_AMOUNT) - return (clampedAmount / MAX_BLUR_RADIUS) * MAX_BLUR_RADIUS + return (clampedAmount / MAX_BLUR_AMOUNT) * MAX_BLUR_RADIUS }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/src/main/java/com/sbaiahmed1/reactnativeblur/ReactNativeProgressiveBlurView.kt` around lines 78 - 88, Update mapBlurAmountToRadius so the normalized blur amount is divided by MAX_BLUR_AMOUNT and then scaled by MAX_BLUR_RADIUS. Preserve the existing invalid-value handling and clamping behavior while ensuring the documented 0–100 input maps correctly to the 0–25 radius range.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@android/src/main/java/com/sbaiahmed1/reactnativeblur/ReactNativeBlurView.kt`:
- Around line 405-410: Update the uniform-corner outline provider in the
isUniform branch to pass topLeft to setRoundRect instead of baseRadius,
preserving explicit equal corner overrides such as zero.
In
`@android/src/main/java/com/sbaiahmed1/reactnativeblur/ReactNativeProgressiveBlurView.kt`:
- Around line 358-367: Correct the horizontal coordinate arrays in the direction
mapping around the leftToRight and rightToLeft branches: assign the gradient’s
transparent and blur endpoints to the same respective edges as the vertical
logic, and apply currentStartOffset to the appropriate endpoint so each
direction renders blur on the intended edge. Leave the vertical fallback
unchanged.
- Around line 78-88: Update mapBlurAmountToRadius so the normalized blur amount
is divided by MAX_BLUR_AMOUNT and then scaled by MAX_BLUR_RADIUS. Preserve the
existing invalid-value handling and clamping behavior while ensuring the
documented 0–100 input maps correctly to the 0–25 radius range.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 733e768a-d6d1-400e-8a3c-a5a765ea75f0
📒 Files selected for processing (5)
android/build.gradleandroid/consumer-rules.proandroid/src/main/java/com/sbaiahmed1/reactnativeblur/ReactNativeBlurSwitch.ktandroid/src/main/java/com/sbaiahmed1/reactnativeblur/ReactNativeBlurView.ktandroid/src/main/java/com/sbaiahmed1/reactnativeblur/ReactNativeProgressiveBlurView.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part 1 of 6 in the July 2026 audit remediation (see
docs/audit-2026-07.md). Stacked onmain.Android correctness, including the one Critical finding.
onDetachedFromWindow, so after react-navigation detach/re-attach (or FlatList cell clipping) it stopped reporting toggles. Teardown now runs only fromonDropViewInstance.consumer-rules.proso a consumer's R8 build doesn't strip the reflected QmBlurView fields and silently regress to whole-screen blur (Blur Effect doesnt work on Android it gives full screen blur effect not parent component #89).Verified on an Android emulator: toggle works, survives navigation, and state persists across a second navigation; no exceptions in logcat.