Skip to content

feat: banner, add optional callback for on animation complete#2758

Merged
brunohkbx merged 6 commits intocallstack:mainfrom
daniel112:banner-on-animation-end-listener
Oct 22, 2021
Merged

feat: banner, add optional callback for on animation complete#2758
brunohkbx merged 6 commits intocallstack:mainfrom
daniel112:banner-on-animation-end-listener

Conversation

@daniel112
Copy link
Copy Markdown
Contributor

@daniel112 daniel112 commented Jun 2, 2021

Summary

After using the Banner component, for better flexibility for customization, I find that it could be useful for consumers to hook into the callback for when the default animation completes.
i.e doing some action after the banner is completely gone as opposed to during animation after setting visible=false

Test plan

I've modified the Banner example to log out whenever it completes the open/close animation. If that is not a preferred modification to examples then you can test by:

  1. modifying BannerExample.tsx for onAnimateShowFinish and/or onAnimateHideFinish callbacks
        <Banner
          ...otherprops
          onAnimateShowFinish={() => console.log('Completed opening animation')}
          onAnimateHideFinish={() => console.log('Completed closing animation')}
        >

      </Banner>
  1. Run the example app
  2. toggle the Banner show/hide
  3. see that it logs out at the appropriate time

@callstack-bot
Copy link
Copy Markdown

callstack-bot commented Jun 2, 2021

Hey @daniel112, thank you for your pull request 🤗. The documentation from this branch can be viewed here.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Aug 2, 2021

Hello 👋, this pull request has been open for more than 2 months with no activity on it. If you think this is still necessary with the latest version, please comment and ping a maintainer to get this reviewed, otherwise it will be closed automatically in 7 days.

@github-actions github-actions Bot added the Stale label Aug 2, 2021
@daniel112
Copy link
Copy Markdown
Contributor Author

@brunohkbx

Copy link
Copy Markdown
Member

@lukewalczak lukewalczak left a comment

Choose a reason for hiding this comment

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

I would rename those

Comment thread example/src/Examples/BannerExample.tsx Outdated
Comment thread src/components/Banner.tsx Outdated
Comment thread src/components/Banner.tsx Outdated
daniel112 and others added 3 commits August 12, 2021 09:49
Co-authored-by: Luke Walczak <lukasz.walczak.pwr@gmail.com>
Co-authored-by: Luke Walczak <lukasz.walczak.pwr@gmail.com>
@daniel112 daniel112 requested a review from lukewalczak August 12, 2021 16:54
@brunohkbx
Copy link
Copy Markdown
Collaborator

hey @daniel112 can you please fix the conflicts?

@daniel112
Copy link
Copy Markdown
Contributor Author

hey @daniel112 can you please fix the conflicts?

done!

@brunohkbx brunohkbx merged commit 14916e3 into callstack:main Oct 22, 2021
@daniel112 daniel112 deleted the banner-on-animation-end-listener branch August 9, 2022 23:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants