-
Notifications
You must be signed in to change notification settings - Fork 640
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
Notification improvements #589
Notification improvements #589
Conversation
vladjerca
commented
Feb 1, 2016
- Moved position to top-right (settings icon is no longer there, so this way the toastr's do not interfere with the application in any way).
- I've changed the remove song toastr to a warning.
- I've changed the color scheme using colors from https://flatuicolors.com/, and also removed rounded edges. (the entire interface is based on strong defined corners, the toastr's just didn't belong).
A screenshot of previous notification and the new one would be helpful. |
It's slightly better now, but still not perfect - now you can't access window action buttons on Windows. @Ajeey |
@@ -23,7 +23,7 @@ app.directive('favoriteSong', function( | |||
SCapiService.deleteFavorite(userId, songId) | |||
.then(function(status) { | |||
if ( typeof status == "object" ) { | |||
notificationFactory.success("Song removed from likes!"); | |||
notificationFactory.warning("Song removed from likes!"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no warning
method in notificationFactory
... it should be noticationFactory.warn()
.
@Pitros Thanks for the screenshots, I was in a bit of a hurry when I made the changes, didn't take the time to take the screenshots. Hope you guys like it as much as I do. Update 1: Further improvements could consist of slightly smaller notifications (width/height) and some padding top to be just under the main navigation bar (illusion of being in the container). Update 2: I've made the changes, the 'warn' method is now used + I've corrected top right margins to offer a more consistent experience: |
I believe if we reduce the size of the image/font-image in the notification toast, it would look more cleaner and compact. |
What were the changes you did to if you updated the code ( |
I used the .less files provided in the official github repositories for each to make the modifications and I just compiled/minified them to replace the old ones. I can just take the diffs and add them to I'll try to tidy up both of the pull requests in the by the end of the week. (some personal matters got in the way) |
I think the changes are good so we just need to revert take your time @vladjerca |
1. Moved position to top-right (settings icon is no longer there, so this way the toastr's do not interfere with the application in any way). 2. I've changed the remove song toastr to a warning. 3. I've changed the color scheme using colors from [https://flatuicolors.com/](url), and also removed rounded edges. (the entire interface is based on strong defined corners, the toastr's just didn't belong).
The commit has been updated. |
@vladjerca thanks but when I said to override I wasn't thinking about adding important all over the place.. What you can do is something like this.. override by selector hierarchy #app { // or body or html instead of #app
// here the toast style without important e.g below
.toast-message a {}
} |
@weblancaster Yeah, my bad on that one. I've updated the scss file. |
@vladjerca below is how looks like now (not as your screenshot).. what you want to do is use you can also move the code below to be inside with the other selectors .toast-message a,
.toast-message label {
font-size: 0.900em;
color: #ecf0f1;
}
.toast-message a:hover {
color: #bdc3c7;
} |
@weblancaster Updated, sorry for all the mishaps. |
Notification improvements