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

Conditional ViewModifier issue #12

Open
vladyslavsosiuk opened this issue Jul 4, 2023 · 0 comments
Open

Conditional ViewModifier issue #12

vladyslavsosiuk opened this issue Jul 4, 2023 · 0 comments

Comments

@vladyslavsosiuk
Copy link

vladyslavsosiuk commented Jul 4, 2023

Hello. Thank you for a great code sample.

I found that it uses a conditional view modifier which is an incorrect implementation in SwiftUI and causes hard-to-track bugs.
The antipattern described in more details here: https://www.objc.io/blog/2021/08/24/conditional-view-modifiers/

Unfortunately, I don't have a possibility to open a PR with a fix, but I can provide suggestions.
Instead of returning modified view if the active argument is true we should pass that argument down to the ShimmerView and compute the duration of the animation based on that argument:

AnimatedMask(
    isActive: isActive,
    phase: phase
).animation(
    .linear(duration: isActive ? duration : 0) // FW: This is a correct way to stop animation. If using `isActive ? .linear(...) : .none` instead then the animation would be choppy. https://stackoverflow.com/questions/61830571/whats-causing-swiftui-nested-view-items-jumpy-animation-after-the-initial-drawi/61841018#61841018
    .repeatForever(autoreverses: bounce)
)

We will also need to retrigger the animation if view didn't reappear but the isActive property flipped from false to true

.onChange(of: isActive) { isActive in
    if isActive {
        phase = 0.8
    } else {
        phase = 0
    }
}

We should pass down the isActive argument down to the AnimatedMask modifier and down to the GradientMaskModifier so that we can compute edgeColor opacity and return 1 if isActive is false.
Let me know if you need more details.

vladyslavsosiuk pushed a commit to vladyslavsosiuk/SwiftUI-Shimmer that referenced this issue Nov 6, 2023
vladyslavsosiuk pushed a commit to vladyslavsosiuk/SwiftUI-Shimmer that referenced this issue Nov 7, 2023
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

No branches or pull requests

1 participant