search

juliancwirko / react-s-alert

Alerts / Notifications for React with rich configuration options
https://www.npmjs.com/package/react-s-alert
MIT License
649 stars 69 forks source link

Add preserveContext option #58

Closed ignatevdev closed 6 years ago

ignatevdev commented 7 years ago

I have tested it on my project, everything seems to work. However, npm run test shows that some unit-tests fail, but it seems like they are failing in the current master as well, so I haven't tried to fix it

ignatevdev commented 7 years ago

Also closes #37

juliancwirko commented 7 years ago

Yes, tests are broken ;) This is not your fault. We need to prepare better testing environment for this lib. There is an 'issue' for that too. Thanks for the PR. I'll test it manually. If this is something which we can enable/disable, and it will not break anything else I think we could merge that, because there was some questions regarding contexts, so it seems that this is needed.

ignatevdev commented 7 years ago

Yes, the new option is disabled by default, so nothing should change for the users after the update unless they turn it on manually. But better merge it only after testing, maybe there is something I've missed.

samvk commented 6 years ago

Solved my issue with not being able to use translations in alerts. Thank you. Hope this is merged into master.

ignatevdev commented 6 years ago

@juliancwirko So can this be merged already? It's been quite a long time

juliancwirko commented 6 years ago

Thanks. Sorry for the delay.