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 75 forks source link

Get `condition` in content template? #50

Closed shakiba closed 6 years ago

shakiba commented 6 years ago

Is there anyway I can get condition in content template? I can extract it from class names passed in props, but if there is anyway to do it without hacking it would be great.

yongzhoulu commented 6 years ago

need this feature

yongzhoulu commented 6 years ago

I made a PR. Feel free to edit it. https://github.com/juliancwirko/react-s-alert/pull/51

shakiba commented 6 years ago

@yongzhoulu Great job!

juliancwirko commented 6 years ago

Hi thanks, can you explain the use case for that?

yongzhoulu commented 6 years ago

In my case. I want different color and different icon inside alert container. default nnn

In this case, i use customTemplate. And i need to know the condition to choose which icon and color i use. You provide the condition api in alertContent in classNames, of couse i can extract from it. It would be easier for me when having condition api. Or i must use customFields to provide this information.

yongzhoulu commented 6 years ago

@juliancwirko hope this scenario is well explained to you

juliancwirko commented 6 years ago

Most of the cases are fulfilled by styling and using provided class names, but yes, I understand that it could be useful when you want to change alert's structure depending on condition.

Ok, I'll work on the lib today and I'll see how we can merge that. Thanks.

yongzhoulu commented 6 years ago

That would be great. Thanks for your time.

3CordGuy commented 6 years ago

Seems like just exposing the condition prop would suffice for most of these cases. What's the status on this? I would just like to have a custom icon as well depending on the called condition method. Seems overkill to have to specify that in a custom field object. :)

juliancwirko commented 6 years ago

Sorry, not ready yet :/ I'll try in the next week.

3CordGuy commented 6 years ago

🤔 #51 is all we need. I guess I'll extract it from the className string until then.

juliancwirko commented 6 years ago

yes, it seems to be the solution here, but I need to spend some time on verification, testing etc.

juliancwirko commented 6 years ago

merged will be published in version 1.4.1 thanks