fix: resolve text clipping and animation lookup issue #1796#1802
Open
xar43 wants to merge 1 commit into
Open
Conversation
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR fixes incorrect animation matching logic (leading to text clipping and wrong animation transfer) by comparing animations by type instead of instance, adjusts provider behavior when no animation is found or when stopping animations, and prevents fixed animation text from clipping horizontally. Sequence diagram for animation lookup with updated equalitysequenceDiagram
participant AnimationBadgeProvider
participant animationMap
participant BadgeAnimation
AnimationBadgeProvider->>AnimationBadgeProvider: getAnimationIndex()
loop for animation in animationMap.entries
AnimationBadgeProvider->>BadgeAnimation: operator==(other:_currentAnimation)
alt [animation.value != null && animation.value == _currentAnimation]
AnimationBadgeProvider->>AnimationBadgeProvider: return animation.key
end
end
AnimationBadgeProvider->>AnimationBadgeProvider: return null
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="lib/providers/animation_badge_provider.dart" line_range="221-225" />
<code_context>
int? getAnimationIndex() {
for (var animation in animationMap.entries) {
if (animation.value != null && animation.value == _currentAnimation) {
+ logger.i("Animation Index: ${animation.key}");
return animation.key;
}
}
- return 0;
+ return null;
}
</code_context>
<issue_to_address>
**question (bug_risk):** Changing the default animation index from 0 to null may break existing callers if they assume a non-null value.
`getAnimationIndex()` used to always return `0` when no animation matched; now it can return `null`. Call sites may currently rely on `0` as a sentinel or use the value without null checks. Please review existing usages to ensure they handle `null` correctly, or define and document a clear convention for when `null` vs `0` should be used to avoid regressions.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+221
to
+225
| logger.i("Animation Index: ${animation.key}"); | ||
| return animation.key; | ||
| } | ||
| } | ||
| return 0; | ||
| return null; |
Contributor
There was a problem hiding this comment.
question (bug_risk): Changing the default animation index from 0 to null may break existing callers if they assume a non-null value.
getAnimationIndex() used to always return 0 when no animation matched; now it can return null. Call sites may currently rely on 0 as a sentinel or use the value without null checks. Please review existing usages to ensure they handle null correctly, or define and document a clear convention for when null vs 0 should be used to avoid regressions.
Contributor
Contributor
Author
|
Hi @ctrlVnt , Thank you for your feedback! I completely understand why it might look like a duplicate at first glance since both address the "Fixed animation" behavior, but I would like to explain the critical architectural differences:
Since this PR completely resolves both the visual clipping math, state management, and the hardware transfer glitch (which was the core of the issue reported here), it offers a permanent, comprehensive fix. I would be extremely grateful if you could review this architectural approach. Thank you so much for your time and guidance! pls look this one also #1785 |
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
2 participants
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.
Closes #1796
Fix "Fixed" Animation Bug
This PR fixes the issue where the Fixed animation doesn't behave as expected. Instead of remaining fixed, it can clip the text and may even get transferred as the Splitting animation.
What was causing the issue?
The app was identifying the selected animation by comparing object instances (
==). This works only when the exact same instance is reused. If a new instance of the same animation class is created, the comparison fails, causing the app to fall back to the wrong animation during transfer.What's changed?
I updated
lib/badge_animation/animation_abstract.dartto make animation comparisons based on the animation type instead of object identity.By overriding
==andhashCodein theBadgeAnimationbase class to compareruntimeType, different instances of the same animation (for example, twoFixedAnimationobjects) are now treated as equal.This ensures the correct animation is recognized and transferred, preventing the unexpected fallback behavior.
WhatsApp.Video.2026-07-07.at.1.10.01.AM.mov
Summary by Sourcery
Adjust badge animation handling to correctly identify and apply fixed animations without clipping or unintended fallbacks.
Bug Fixes: