Logs

[UziTech]

Description of the Change

Add a notification log.

I added a working version of the sm-logs branch.

Todo:

• [x] Show notifications in log
• [x] Command to toggle log
• [x] Use dock instead of panel
• [x] Click to expand notification https://github.com/atom/notifications/issues/29#issuecomment-262122434
• [x] Sort by/Hide categories
• [x] Show buttons?
• [ ] Counter on status bar https://github.com/atom/notifications/issues/29#issuecomment-110062482
• [ ] Disable popups setting (in another pull request)
• [x] Timestamps

Alternate Designs

As I've been writing this I've realized this might be better as an additional package.

The way notifications are re-shown would require this to be integrated.

I tried initially just expanding the notification in the log instead of showing it as a pop up but the logic to render fatal errors would need to be reused. It was easier to re-open the pop up notification.

Benefits

See past notifications.

Possible Drawbacks

none

Applicable Issues

29

[UziTech]

This also seems to fix #110 so we wouldn't need pull request #163

I re-show the notifications like @simurai showed in https://github.com/atom/notifications/issues/29#issuecomment-262122434

[winstliu]

Excited for this.

[winstliu]

Hmm...maybe I broke it? Looks like the notification got cut off (editor:log-cursor-scopes).

[UziTech]

nice catch. looks like I will have to figure out what to do with html in the message

[UziTech]

I would like some feedback on a few design decisions:

1. Should the most recent notification be at the top or bottom?

   I'm leaning toward the top since the most common use of the log is to look at a recent notification, but that kind of seems unintuitive.

2. Should I show the whole message in the case of a multi-line message or limit it to one line?

   I think one line would work as a summary since they can open the notification to see more.

3. Should I show buttons in the log?

   My only concern is that fatal buttons might change after the issue is checked. I'm not sure how to get the updated button.

   Also if the log is in the left or right dock there isn't much room for the buttons.

4. Should I include a timestamp with the notifications in the log? maybe as a tooltip?

/cc @50Wliu @simurai

[winstliu]

Should the most recent notification be at the top or bottom?

I'm also leaning towards the top.

Should I show the whole message in the case of a multi-line message or limit it to one line?

Summary sounds reasonable.

Should I show buttons in the log?

This would be :cool: if the button updating thing can be worked out.

Should I include a timestamp with the notifications in the log? maybe as a tooltip?

Also cool. If there's enough space maybe it can go on the far right.

[UziTech]

I think this is done (except for tests) I'd like to do the status bar counter and disable popups setting in a separate pull request

[winstliu]

What do you think about using something like moment.js for handling the relative time?

[UziTech]

That could work. I've never dealt with relative time before.

[UziTech]

I noticed in a couple other languages that the timestamp will overflow the box when it is under a minute.

I'm not sure if the best solution is to just let it over flow so they can see part of it, or expand the box so it isn't aligned with the other timestamps.

[UziTech]

I think I solved it by removing the border on the left and re-working how the buttons and timestamp are laid out

[simurai]

@UziTech Should the most recent notification be at the top or bottom? @50Wliu I'm also leaning towards the top.

Another vote for top.

@UziTech Should I show the whole message in the case of a multi-line message or limit it to one line? @50Wliu Summary sounds reasonable.

Yeah, one line should be fine and make it easier to scan. Maybe, it could be expanded as an alternative to open a "popup" notification, but out of scope for this PR.

@UziTech Should I show buttons in the log? @50Wliu This would be :cool: if the button updating thing can be worked out.

Should the buttons be left aligned, meaning they would show up right after the text:

Before:

After:

They won't align, since everything has a dynamic width, but clashes less with the "time ago". Or table columns?

@UziTech Should I include a timestamp with the notifications in the log? maybe as a tooltip? @50Wliu Also cool. If there's enough space maybe it can go on the far right.

👍

[UziTech]

@simurai it looks like the transparent top border is making the icon border perforated

[simurai]

@UziTech it looks like the transparent top border is making the icon border perforated

👍 Right. Ok, switched to box-sizing: content-box; instead of the border.

[UziTech]

This is ready to be reviewed

[winstliu]

I'm starting to suspect this is something with my laptop, but whenever I close the developer tools the Logs dock opens up. Basically the same as https://github.com/atom/github/issues/753.

[UziTech]

I have quite a few updates to this package (status bar notification count, timeout settings, etc.) that require this pull request to be merged in order to work.

I created a new package with the latest updates since I know it can take a while for a core package to be updated.

notifications-plus

[simurai]

@UziTech Ok, gonna try out notifications-plus's 🙈 hard-core mode:

[UziTech]

I use it like that with notifications-plus-confetti so I can still see when I get a notification and can click on the count to open the log if I actually want to see what the notification was about. 😃

[winstliu]

The log doesn't seem to be too happy when in a side dock.

[UziTech]

@50Wliu how would you prefer it be displayed in the side dock?

I think all the necessary information is there:

1. The type of notification
2. The start of the message
3. At least the first button
4. the time (maybe we could shorten this)

This is really only supposed to be a synopsis. If they want to see more of the notification all they have to do is click on it.

I feel like the most important information in this view would be the time and type of notification.

[winstliu]

I noticed that when you click on a temporary notification to make it permanent, core:cancel does not hide the notification.

[UziTech]

@50Wliu I fixed it.

The problem was that only initially dismissable notifications were subscribing to onDidDismiss. Now all notifications subscribe to onDidDismiss

[ungb]

I went through and tested this, I definitely see this as a huge improvement. One thing I think would be nice to have is a way to clear the logs without reloading atom, but I think this is a nice to have.

Otherwise it looks like there is a merge conflict. Once this is resolved I think we should be good to go. Looks like @50Wliu has already reviewed the code.

/cc @iolsen to help get this thru the 🚪

[winstliu]

I'd like to point out that my review was mostly superficial and that I didn't feel knowledgeable enough to review notifications-log-item.

@UziTech would you mind resolving conflicts?

[UziTech]

Merge conflicts taken care of. 👍 +1 on the clear logs. I could add a command notifications:clear pretty easily if more people are interested.

It would be kind of nice to make atom.notifications.clear() public and have a did-clear-notifications event emitted

[UziTech]

@ungb I added a button to clear the notifications on notifications-plus

Also submitted a pull request to make atom.notifications.clear() public and emit an event https://github.com/atom/atom/pull/16074

[UziTech]

@50Wliu is someone reviewing this?

[leroix]

👋 @UziTech I gotcha fam. Catching up on your changes.

Can you help me understand the problem that this is solving a bit better? What are some situations where not having a notification log becomes particularly painful?

[UziTech]

@leroix thank you for taking this on.

This pull request helps a few things:

• Reshow a notification that gets auto-dissmissed before you can finish reading it
• Click on an auto-dissmissing notification to prevent it from auto-dissmissing
• Deal with a notification when you have time without keeping it open

[UziTech]

Ya I have been using it for a while and it has been working great. Let's ship it 👍