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

Custom Background Color and swipe up #11

Closed
wants to merge 2 commits into from
Closed

Conversation

xremix
Copy link

@xremix xremix commented Mar 29, 2018

Hey

this library works great for my project. However the styles don't fit the theme of my App. So I build a little extension that allows to pass a custom background color.

This is more a solution to get my problem fixed. A better solution might be, to let the CRNotificationType be a Class that include the color (and maybe border, shadows, etc.). There could be a couple of static variables that return the defaults for info, error, etc., that will be passed as a parameter, but user could still create a new instance with custom properties.
I could work on something like this, if you see this benefitial.

@xremix
Copy link
Author

xremix commented Mar 29, 2018

Also, would it make sense to let the CRNotification class be public, so people could create custom instances or write their own showNotification() function?

@xremix xremix changed the title Custom Background Color Custom Background Color and swipe up Mar 29, 2018
@dkcas11
Copy link
Owner

dkcas11 commented Mar 29, 2018

Hey, thanks for your PR 🙂

I have actually had the intention of adding this myself but I never got to do it. (I am not sure if I actually did it, but never put it up here)

The way to do this would be to just take a protocol as a parameter which any struct or class can implement, returning the correct image and colours.

The project could use a little spring cleaning, so I might as well do that to it.

I like the addition of the swipe gesture.

@anthonycastelli
Copy link

What is the status of this PR? I like the idea of the Swipe, and the ability to change colors would be ideal.

@dkcas11
Copy link
Owner

dkcas11 commented Apr 16, 2018

@anthonycastelli I have the project on my Mac where this is done and where I have expanded on the implementation to allow for better custom styling. I have yet to decide on further additions but I can put it up here tomorrow and you guys can come with some inputs on that maybe.

@xremix
Copy link
Author

xremix commented Apr 17, 2018

@dkcas11 I was trying to implement an solution by using a protocol, but had some issues with that, so I didn't commit these changes.
I'd love to review your changes and give you my input. This is really beneficial to me.

@xremix
Copy link
Author

xremix commented Apr 17, 2018

@dkcas11 do you want me to open a separate PR for the swipe up?

@dkcas11
Copy link
Owner

dkcas11 commented Apr 17, 2018

@xremix I have done the exact thing, and implemented a protocol for anyone to implement and customize their notifications. This wasn't the intention originally, since i just wanted it to be easy to use, but what i really just got was the shortcut like .success when passing a notification type. So this now needs to be passed as CRNotifications.success or something like that.
As i said, i have it on my Mac at home (on the working MBP right now), so i will put up the PR in about an hour once i am home and ping you 🙂

@dkcas11
Copy link
Owner

dkcas11 commented Apr 17, 2018

@xremix PR is up 👍

@dkcas11
Copy link
Owner

dkcas11 commented Jul 11, 2018

Now implemented (not from this PR).

@dkcas11 dkcas11 closed this Jul 11, 2018
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.

None yet

3 participants