Opened 11 years ago
Closed 11 years ago
#706 closed enhancement (fixed)
New notifications
| Reported by: | joar | Owned by: | |
|---|---|---|---|
| Priority: | major | Milestone: | 0.5.0 |
| Component: | programming | Keywords: | |
| Cc: | Parent Tickets: |
Description
I have been working on a new generation of notifications in MediaGoblin: https://github.com/joar/mediagoblin/tree/notifications (comparison: https://github.com/joar/mediagoblin/compare/notifications).
It's come a long way, but still things to figure out. This ticket has been created to track that conversation and development.
Change History (13)
comment:1 by , 11 years ago
| Owner: | removed |
|---|---|
| Status: | new → review |
follow-up: 3 comment:2 by , 11 years ago
| Owner: | set to |
|---|---|
| Status: | review → in_progress |
comment:3 by , 11 years ago
| Owner: | removed |
|---|---|
| Status: | in_progress → review |
My notifications branch has been rebased and contains only two commits at the moment.
Replying to cwebber:
Heya Joar,
I've read/reviewed the code, but very briefly! It looks good. Here's some comments:
- This is an important new piece of infrastructure... as such, it could really use tests.
Added basic test which should cover at least the most common usage.
- The javascript needs copyright headers.
Fixed
- One commit is marked "temporary" and another marked "misc"... they both look more useful than that... I wonder how impossible it would be for you to back-edit the commits to give more useful messages? :)
Rebased entire branch into two commits.
- Before I could suggest you use celery, you are already using celery. Good work! ;)
Thanks ;)
- For some reason I'm having trouble figuring out how to get notifications. :) I've subscribed to notifications on a couple of posts, I make comments with various users, nothing shows up! :)
I have no idea, a database dump might help debugging in the future :)
Lastly some comments about the UI, from what I can see at present:
- I'm excited generally! I wonder if having two buttony things in the corner is best though. I had a conversation with Deb where she suggested that when logged in that we have a button that looks like:
[cwebber :V]Instead of:
[V]Expanding on that, how about when we have no notifications, instead of 0, we just nave nothing. Then we could either do:
[2| cwebber :V] *OR* [cwebber :2] *OR* [2] [cwebber:V]That may not be for the best, I don't know! I do think if there are no notifications, it shouldn't show up though. No need to make someone look at something when there's nothing to look at? Or just have it be empty until there's something to go there.
I have made it so that the notification-gem button-y thing does not show up if there are no notifications.
- I'd make comments about the "viewing notifications" bit, but I can't presently view them, so I'll wait till you help me figure out what I'm doing wrong :)
Overall, I'm really excited about the direction and work you've put into this! I think this won't land for 0.4.0, but I'm really hoping we get it in 0.4.1!
\o/
comment:6 by , 11 years ago
| Owner: | set to |
|---|---|
| Status: | review → in_progress |
Okay, AWESOME! I've merged this. That click-through to the comment thing is just awesome. That glowy comment thing: JUST AWESOME! I love it.
I think this was already good to be merged, but before the next release, there's a major feature missing I think... there's presently no way to mark messages as read from the interface itself. You have to click through to each notification. This is a bit painful! I think we need a way to mark messages as read individually or as a group *without* jumping to the individual things. It's a bit painful otherwise.
Joar, I'm passing this back to you because I think that your input there would be helpful, and maybe you'd want to work on it? (If not, you can unclaim it of course.)
GREAT WORK Joar! This is an awesome feature.
follow-up: 8 comment:7 by , 11 years ago
Two more comments:
- It would be awesome to have notifications when things finish processing for big media... it would be annoying for images, but would be really helpful for video and probably audio. This should probably be its own ticket though.
- I wonder about people who do not want to get these notifications? Maybe we could have a user preference, and maybe also a config option to turn it off on your site altogether? What do you think?
comment:8 by , 11 years ago
Replying to cwebber:
Two more comments:
- It would be awesome to have notifications when things finish processing for big media... it would be annoying for images, but would be really helpful for video and probably audio. This should probably be its own ticket though.
To communicate on one of your points:
I think we should notify for all media, making sure that there's no obfuscation about what's happening behind the scenes, and not showing image stats might be annoying if the server is really slow.
To make it overviewable we should keep the notifications as simple as possible, they should not be taking over the UI, and if we have a page that shows all processing notifications it should group similar notifications together and only show a summary of them unless explicit step is taken to show all of them
comment:9 by , 11 years ago
Okay, fair enough re: all media.
I still think cleaner ways to clear this stuff is critical before we make the next release.
Joar, are you thinking of working on that this release?
comment:11 by , 11 years ago
| Status: | accepted → review |
|---|
So I added the ability to mark all comment notifications read. https://github.com/rodney757/mediagoblin/tree/notifications
comment:12 by , 11 years ago
added a user preference for notifications, but it sounded like we weren't sure if we wanted this. Feel free not to merge that commit :)
comment:13 by , 11 years ago
| Resolution: | → fixed |
|---|---|
| Status: | review → closed |
Looks great. I merged it! Nice work Joar and Rodney on this! Woohooooooooo!
Heya Joar,
I've read/reviewed the code, but very briefly! It looks good. Here's some comments:
Lastly some comments about the UI, from what I can see at present:
[2| cwebber :V] *OR* [cwebber :2] *OR* [2] [cwebber:V]Overall, I'm really excited about the direction and work you've put into this! I think this won't land for 0.4.0, but I'm really hoping we get it in 0.4.1!