bayleedev / electron-notifications

:boom: A node module for sending notifications in electron applications.
MIT License
133 stars 54 forks source link

Adding some rudimentary stacking #10

Closed davidcoleman007 closed 7 years ago

davidcoleman007 commented 7 years ago

Hi guys, I've made this PR with my stacking mods as promised.

bayleedev commented 7 years ago

This is awesome! I'll try and look at this soon. :D

bayleedev commented 7 years ago

What's the behavior when it gets to the bottom of the screen? It seems those stack:

screenshot_12_9_16__3_24_pm

I set the timeout to 10 seconds, I opened 4 with a few seconds delay between each of them. When two closed, I click the button to open another and it opens in the wrong spot. You can see the first one here is overlapping:

screen shot 2016-12-09 at 3 26 09 pm

After all 4 close, I click the button again, and it appears in the second position.

davidcoleman007 commented 7 years ago

Yeah, that’s something that I mentioned in a TODO… I don’t have any collapsing when a middle one is removed yet. It’s a work in progress, which I’ll be working on for my personal project and then contribute here. Will probably use jquery for a nice slide up/down effect and also utilize the maxStack property that I just have hanging in the constructor, then collapse things and pop off the stack of pending stuff which I didn’t build yet.

Also want to allow it to be from top or bottom depending on OS, so that it looks native to windows and mac peeps alike. :)

As I said, this is “rudimentary” stacking. :)

I intend to build this out much more over the next week or so as our project progresses.

From: Blaine Schmeisser [mailto:notifications@github.com] Sent: Friday, December 9, 2016 3:27 PM To: blainesch/electron-notifications electron-notifications@noreply.github.com Cc: David Kenneth Coleman david_coleman_007@hotmail.com; Author author@noreply.github.com Subject: Re: [blainesch/electron-notifications] Adding some rudimentary stacking (#10)

What's the behavior when it gets to the bottom of the screen? It seems those stack:

https://cloud.githubusercontent.com/assets/769039/21067959/8bea30c6-be23-11e6-9d21-cf28b5aeb7c0.png

I set the timeout to 10 seconds, I opened 4 with a few seconds delay between each of them. When two closed, I click the button to open another and it opens in the wrong spot. You can see the first one here is overlapping:

https://cloud.githubusercontent.com/assets/769039/21068000/daafc77a-be23-11e6-84d1-d597ba113224.png

After all 4 close, I click the button again, and it appears in the second position.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/blainesch/electron-notifications/pull/10#issuecomment-266151217 , or mute the thread https://github.com/notifications/unsubscribe-auth/AACgcxy98ZRnlFNrMEboUbVQf_GFkBfaks5rGePYgaJpZM4LHZ1A . https://github.com/notifications/beacon/AACgc2MNClyJEckZQXgteKhdZU3lFRbIks5rGePYgaJpZM4LHZ1A.gif

davidcoleman007 commented 7 years ago

Hi @blainesch I've updated this with the following changes:

I wanted to do some jQuery to make them fancy and collapse with everything all slick... that was a lot of work that seemed be out of place as an initial implementation. I focused on making the stack WORK.

davidcoleman007 commented 7 years ago

I should mention that I recently noticed that on windows the notifications are MOVABLE if I click and drag on the body of the note... maybe this shouldn't be possible?

davidcoleman007 commented 7 years ago

New stacking playbook entry: image

davidcoleman007 commented 7 years ago

This is what a maxStack == 7 looks like: ignore the start bar overlap, b/c I have a 3 row start bar since I run on 3x QHD monitors and have a tall start bar on the middle screen. 75 px should be just fine on any std windows system. when we have a more elegant way to detect the size of the start bar, we can use it.

image

davidcoleman007 commented 7 years ago

also I didn't dig down into the css yet, but I feel like the gap should be configurable. Just a thought.

maybe

notifier.configure({
    maxStack : 7,
    stackGap: 10
});
bayleedev commented 7 years ago

The max stack should default based on the screen size.

davidcoleman007 commented 7 years ago

I like that, and agree 100%. yes I'll do it in the next few days!

davidcoleman007 commented 7 years ago

ok, new plan. I'll do it tonight! :)

davidcoleman007 commented 7 years ago

@blainesch is there some reason you have held off on this PR? Is there some additional issue which you feel to be required?

bayleedev commented 7 years ago

Oh sorry. I saw your comment, but didn't see the commit. I'll take a look. Thank you for reminding me [:

bayleedev commented 7 years ago

This is great, good job! [:

bayleedev commented 7 years ago

Published as v0.1.5