search

aagarwal1012 / Animated-Text-Kit

🔔 A flutter package to create cool and beautiful text animations. [Flutter Favorite Package]
https://animated-text-kit.web.app
MIT License
1.65k stars 305 forks source link

setState text workaround solution added in README #243

Closed Mohitmadhav closed 3 years ago

Mohitmadhav commented 3 years ago

Fixes #198

  1. Wrote the setState text issue in the README file.
  2. Added the solution (a key that changes based on the text) along with the video by Google Developers.

Please tell if anything else needs to be added - Like flutter documentation, etc.

codecov[bot] commented 3 years ago

Codecov Report

Merging #243 (ccd97bf) into master (f50ac48) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #243   +/-   ##
=======================================
  Coverage   95.38%   95.38%           
=======================================
  Files          10       10           
  Lines         455      455           
=======================================
  Hits          434      434           
  Misses         21       21

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f50ac48...ccd97bf. Read the comment docs.

Mohitmadhav commented 3 years ago

I updated the README 2 minutes ago. Please check @aagarwal1012 @SirusCodes.

aagarwal1012 commented 3 years ago

@all-contributors add @Mohitmadhav for documentation.

allcontributors[bot] commented 3 years ago

@aagarwal1012

I've put up a pull request to add @Mohitmadhav! :tada:

SirusCodes commented 3 years ago

Hey @aagarwal1012 have you seen my comment on?

aagarwal1012 commented 3 years ago

@SirusCodes where is your comment?

SirusCodes commented 3 years ago

Actually I started a review Screenshot_20210429-005612~2.png

aagarwal1012 commented 3 years ago

@SirusCodes why your comment doesn't appear in this PR?

aagarwal1012 commented 3 years ago

Anyways, @Mohitmadhav can you create a new PR stating @SirusCodes' comments?

SirusCodes commented 3 years ago

Maybe starting a review won't appear on PR.🤔

Mohitmadhav commented 3 years ago

@SirusCodes so what more possible reasons could be there?? Even I am confused. Then only I'll be able to add an example with reference to that in the new PR.

SirusCodes commented 3 years ago

So, I tried to replicate the issue.. and observed a few things and found out the reason but I don't know exactly why keys fixes it. My 1st guess would be what that video says...

What causes the issue: When user calls setState the tree changes and new text list is assigned but the _index field in the AnimatedTextKit is not updated according to new list size. That is if the user calls setState after the index is crossed it will throw RangeError.

Example

  1. text has 3 strings
  2. Animation completes till 2nd index
  3. User calls setState
  4. Now text length is 2 but index is 2 (on 3rd text)
  5. bool get _isLast => _index == widget.animatedTexts.length - 1; this condition fails and index is increased.
  6. Which asks for index location 3 (4th text) which is not there in List hence causes the error.

Possible fix bool get _isLast => _index == widget.animatedTexts.length - 1; => bool get _isLast => _index >= widget.animatedTexts.length - 1; This might fix the issue but testing is required...

If this issue is resolved by this then there is not need to add this workaround ;)

SirusCodes commented 3 years ago

@aagarwal1012 and @Mohitmadhav take a look at it