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

fix: AnchoredOverlayKey key init #486

Merged
merged 3 commits into from
Dec 3, 2024

Conversation

vongrad
Copy link
Contributor

@vongrad vongrad commented Nov 10, 2024

Description

ShowCaseWidgetState initializes the anchoredOverlayKey in initRootWidget after the first frame was rendered (WidgetsBinding.instance.addPostFrameCallback). This means that the Showcase widget's AnchoredOverlay will initially have the key property set to null.

This causes unwanted behavior since when the key is eventually updated in the AnchoredOverlay, it will cause its child to get disposed and completely rebuilt. In my case, the child I pass into the Showcase widget and (so the AnchoredOverlay) is animated and disposing the child breaks its animations.

I suggest that the anchoredOverlayKey in the ShowCaseWidgetState is initialized directly in the initState or if there is some reason for it to be initialized after the first frame (which I do not believe so), make it part of the _InheritedShowCaseView and let the Showcase widget pass the update to the AnchoredOverlay using ShowCaseWidget.of....

Checklist

  • The title of my PR starts with a Conventional Commit prefix (fix:, feat:, docs: etc).
  • I have followed the Contributor Guide when preparing my PR.
  • I have updated/added tests for ALL new/updated/fixed functionality.
  • 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.
  • No, this PR is not a breaking change.

Related Issues

Copy link
Collaborator

@Sahil-Simform Sahil-Simform left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, @vongrad , for raising this PR! It looks great overall. Could you please incorporate the suggested changes? Let me know if you have any questions or need clarification on the feedback. Looking forward to the updated version!

lib/src/showcase_widget.dart Outdated Show resolved Hide resolved
Copy link
Collaborator

@Sahil-Simform Sahil-Simform left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update, @vongrad ! I have one small suggestion. Could you please make the update so we can move forward?

lib/src/showcase_widget.dart Outdated Show resolved Hide resolved
Copy link
Collaborator

@Sahil-Simform Sahil-Simform left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@aditya-css aditya-css merged commit 433095c into SimformSolutionsPvtLtd:master Dec 3, 2024
2 checks passed
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

Successfully merging this pull request may close these issues.

3 participants