tanmayrajani / notifications-preview-github

Browser Extension: preview GitHub notifications with same page pop-overs
https://chrome.google.com/webstore/detail/github-notifications-prev/kgilejfahkjidpaclkepbdoeioeohfmj?hl=en&gl=IN
MIT License
141 stars 15 forks source link

Feature request: Add ability to change polling interval? #126

Open vfonic opened 2 years ago

vfonic commented 2 years ago

Hi!

Great extension! This should be GitHub's native functionality IMO.

I noticed that this extension pings GitHub for notifications every 5 seconds. I don't want this to happen so often. I'd prefer 60 seconds. 5 seconds is great if I need something close to real-time. I actually use this extension because of the nice popover which allows me to avoid having to click on the bell icon and be redirected to a whole new page (I can just CMD+Click on notification in popover and open notification in a new tab)

What happens, for example, if I have GitHub website opened in 20 tabs? Will I be polling GitHub from each one of these tabs until the browser makes them inactive?

Is there some way to change this? Is this something you'd consider adding as an option?

Thank you! 🙏

tanmayrajani commented 2 years ago

What happens, for example, if I have GitHub website opened in 20 tabs? Will I be polling GitHub from each one of these tabs until the browser makes them inactive?

hey, no, it will only poll notifications on currently focused tab


but yeah, I guess this might be a good idea, letting users choose the duration. @fregante WDYT? I guess we could do it if more people want it?

vfonic commented 2 years ago

Basically, I noticed this while reviewing a PR and I couldn't forget that this request is being made every 5 seconds. :D

fregante commented 2 years ago

We could also stop the request from happening unless the user is within the first 300px of scrolling. There's no point in fetching it if one can't even see it.

As for the implementation of the requested option we can use an input type=range between 5 and 60

vfonic commented 2 years ago

That's interesting idea. Although I wouldn't say 300px of scrolling, as user could have some custom CSS script installed (I do), which could keep the header sticky on top (ok, I don't have this :)).

Maybe better would be "if cursor is within 300px (absolute distance) from the bell icon" 🤷‍♂️

I'd say that just adding a config option for the polling interval would greatly improve "UX" and could be a quick win, while @fregante's suggestion could be more enhanced functionality, a v2 so to say.

Araxeus commented 2 years ago

has anyone started working on this and has any UI prototype specifically?

I might start working on this.

here's an idea for waiting until the notification icon is almost visible (viewport +100 px on Y axis) https://developer.mozilla.org/en-US/docs/Web/API/Intersection_Observer_API

let observer = new IntersectionObserver(toggleUpdateLoop, {
    rootMargin: '100px 0 0 0'
});
observer.observe(select('.notification-indicator'))

here are the section to be changed:

https://github.com/tanmayrajani/notifications-preview-github/blob/4cb8317ccf87f36cf7081ffa6a108aa76ba2b2a7/source/github-notifications-preview.jsx#L193 https://github.com/tanmayrajani/notifications-preview-github/blob/4cb8317ccf87f36cf7081ffa6a108aa76ba2b2a7/source/libs/utils.js#L6-L8 https://github.com/tanmayrajani/notifications-preview-github/blob/master/source/options.html