Skip to content
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

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

Open
vfonic opened this issue Jan 16, 2022 · 5 comments
Open

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

vfonic opened this issue Jan 16, 2022 · 5 comments

Comments

@vfonic
Copy link

vfonic commented Jan 16, 2022

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
Copy link
Collaborator

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
Copy link
Author

vfonic commented Jan 16, 2022

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

@fregante
Copy link
Owner

fregante commented Jan 16, 2022

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
Copy link
Author

vfonic commented Jan 16, 2022

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
Copy link

Araxeus commented Apr 19, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants