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: remove delay when clearing all notifications #342

Merged
merged 7 commits into from
Oct 21, 2024

Conversation

rubiin
Copy link
Contributor

@rubiin rubiin commented Oct 20, 2024

When having 50+ notifications, clicking the clear all button takes some time before it does the job.

This was because we were adding a delay with settimeout of 100 ms .
This makes the clearall instant which is a better UX similar to other implementations like in swaync

@rubiin rubiin force-pushed the close-notification branch from f366843 to c7a1b8c Compare October 20, 2024 12:36
@Jas-SinghFSU
Copy link
Owner

I'm not a fan of the delay either ideally it should be MUCH lower but this is something you have to do to prevent crashes in AGS. Notice the clearDelay property on the AGS documentation https://aylur.github.io/ags-docs/services/notifications/.

For some users this number has to be even higher. If we want to remove it, we can add an option to let the user define the delay amount and add a warning below the settings that "Setting this value too low may crash AGS".

@rubiin
Copy link
Contributor Author

rubiin commented Oct 20, 2024

Looks like this PR still needs some work. I will try to add in the option by tomorrow

@rubiin rubiin force-pushed the close-notification branch from c7a1b8c to c0d3bfd Compare October 21, 2024 02:28
@rubiin
Copy link
Contributor Author

rubiin commented Oct 21, 2024

Updated the setting widget to include the delay with the initial value as 100 ms.

@Jas-SinghFSU Jas-SinghFSU changed the title fix: remove delay when clearing all notifications feature: remove delay when clearing all notifications Oct 21, 2024
@Jas-SinghFSU Jas-SinghFSU merged commit 3bc8c0d into Jas-SinghFSU:master Oct 21, 2024
1 check passed
@Jas-SinghFSU
Copy link
Owner

That's for the wonderful contributions! ❤️

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

Successfully merging this pull request may close these issues.

2 participants