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

Added a couple of new options #898

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fbandrei
Copy link

Added a couple of new options

  1. HTML for the close button
  2. Options for selecting the type of ease (one for showing and one for
    hiding)
  3. Options to configure the ease time (in and out)

I added these options in the demo example as well, please check them out.

Feel free to change the namings as you wish.

@@ -47,7 +50,7 @@ import { ToastrService } from './toastr.service';
),
transition(
'active => removed',
animate('{{ easeTime }}ms {{ easing }}')
animate('{{ hideEasingTime }}ms {{ hideEasing }}')
Copy link
Owner

Choose a reason for hiding this comment

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

i think this is a breaking change since anyone that has set the easeTime would now get the default

Copy link
Owner

Choose a reason for hiding this comment

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

should keep the names similar hideEaseTime

1. HTML for the close button
2. Options for selecting the type of ease (one for showing and one for
hiding)
3. Options to configure the ease time (in and out)

The new hideEaseTime property is only used if it's defined, otherwise a
fallback to easeTime is done so that there is no breaking change.
@fbandrei
Copy link
Author

fbandrei commented Sep 1, 2021

I fixed that breaking change by adding a fallback to the easeTime property if the new hideEaseTime property is not defined. Thus, there is no breaking change any more.

@jcompagner
Copy link

can this pull request be integrated?

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