flame icon indicating copy to clipboard operation
flame copied to clipboard

feat: Loading builder for Route

Open PistonShot1 opened this issue 1 year ago • 3 comments

Description

This change is mainly to make it easier for specifying loading screen along when routing between component using the RouterComponent. Currently the change is only made on Route, that now have optional loadingBuilder to Route constructor which would expect a Component, which would be added as the loading page/screen while onLoad of the builder (the main routing component) to be completed.

Checklist

  • [x] I have followed the Contributor Guide when preparing my PR.
  • [ ] I have updated/added tests for ALL new/updated/fixed functionality.
  • [x] I have updated/added relevant documentation in docs and added dartdoc comments with ///.
  • [ ] I have updated/added relevant examples in examples or docs.

Breaking Change?

  • [ ] Yes, this PR is a breaking change.
  • [x] No, this PR is not a breaking change.

PistonShot1 avatar Apr 03 '24 08:04 PistonShot1

wait ya , dont merge yet, adding test for this

PistonShot1 avatar Apr 04 '24 09:04 PistonShot1

Don't merge yet.

hi @spydon , sorry I was away for awhile. Just pushed new one with test. Anyways, I just have problem it making a full test to check if the loading component was mounted before it was removed. I was only able to check for loading component was removed and the builder component ( page component) was mounted right after it was removed.

If you look at the file route_test.dart , I added test and also did add few comments , to explain the situation for the test coverage. Sorry I am not that good at coming with test, but I did manually check it by linking my version of flame repo to one of my old flame game, I mean my flame in pubspec refer to my git repo instead of pub.dev and test this feature I added, according to my observation it did work to what I expected.

PistonShot1 avatar Apr 12 '24 10:04 PistonShot1

I mean does checking the loading component was removed technically means it also verifies that it was mounted before right . So if that is the case then the test I added already suffice ?

PistonShot1 avatar Apr 12 '24 11:04 PistonShot1

@PistonShot1 have you made any progress on this? :)

spydon avatar Jul 21 '24 10:07 spydon

@spydon . I have fixed it, sorry took so long for such a small feature addition, I gave up after the test keep on fail, but as you suspected it was the way I wrote the test, now it works fine and should be working and passed the expected tests

PistonShot1 avatar Jul 24 '24 12:07 PistonShot1