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(modal): ModalDirective should use config.animated #3156

Merged

Conversation

flensrocker
Copy link
Contributor

@flensrocker flensrocker commented Dec 1, 2017

As the comment in the source says "isAnimated seems like an Options" it actually is an option, which should be changeable with a ModalOptions provided with the config input.

Closes #3137

@valorkin
Copy link
Member

valorkin commented Dec 1, 2017

well done! thanks
QA will take a look on monday, and I will be able to merge

@valorkin valorkin changed the title ModalDirective should use config.animated fix(modal): ModalDirective should use config.animated Dec 1, 2017
@codecov
Copy link

codecov bot commented Dec 1, 2017

Codecov Report

Merging #3156 into development will increase coverage by 0.01%.
The diff coverage is 0%.

Impacted file tree graph

@@              Coverage Diff               @@
##           development   #3156      +/-   ##
==============================================
+ Coverage        65.29%   65.3%   +0.01%     
==============================================
  Files              209     209              
  Lines             5642    5641       -1     
  Branches           994     994              
==============================================
  Hits              3684    3684              
+ Misses            1695    1694       -1     
  Partials           263     263
Impacted Files Coverage Δ
src/modal/modal.directive.ts 20.34% <0%> (+0.11%) ⬆️

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 aa683a7...987f1d9. Read the comment docs.

@YevheniiaMazur
Copy link

Tested, looks good

@valorkin valorkin merged commit f5679eb into valor-software:development Dec 4, 2017
@ghost ghost removed the ready for merge label Dec 4, 2017
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.

3 participants