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

Fix notifications on all services #1593

Merged
merged 5 commits into from May 13, 2024

Conversation

SpecialAro
Copy link
Member

@SpecialAro SpecialAro commented Feb 25, 2024

Pre-flight Checklist

Please ensure you've completed all of the following.

Description of Change

Change notification class to extend window.Notification.

Motivation and Context

Several users are reporting that notifications don't work on several services (noticed that icon was not being shown properly)
Maybe fixes #1105
Fixes #1751
Fixes #1747
Fixes #896
Fixes #1719
Fixes #1652

Steps to fix this PR:

  • Fix the onClick function (not working since ever....) to properly open Ferdium (if closed), open the service, and go to the corresponding message on a service (new feature). (I was not able to implement this last logic... it's rather complex. Leaving for the future).
  • For some reason, this new approach is spawning two notifications (I think that the first one is from window.Notification natively and that the other one is our class Notification). Nevertheless, clicking on the latest notification closes all notifications. Need help fixing this (@vraravam if you can take a look at this please) - or should we merge this PR with a "Known issue" for this?

Checklist

  • My pull request is properly named
  • The changes respect the code style of the project (pnpm prepare-code)
  • pnpm test passes
  • I tested/previewed my changes locally

Release Notes

Fixed several issues with notifications (double notifications, no icons on notification, content showing when private).
Added feature to open Ferdium and go to the specific service after clicking the notification.

@SpecialAro SpecialAro changed the title Fix notifications on all services WIP: Fix notifications on all services Feb 25, 2024
ponces pushed a commit to ponces/ferdium-app that referenced this pull request Apr 30, 2024
ponces pushed a commit to ponces/ferdium-app that referenced this pull request Apr 30, 2024
ponces pushed a commit to ponces/ferdium-app that referenced this pull request May 3, 2024
ponces pushed a commit to ponces/ferdium-app that referenced this pull request May 3, 2024
ponces pushed a commit to ponces/ferdium-app that referenced this pull request May 3, 2024
@SpecialAro
Copy link
Member Author

Self-reminder: This PR should also address the issue here (#1751) that was introduced by the extend window.Notifications

ponces pushed a commit to ponces/ferdium-app that referenced this pull request May 8, 2024
@SpecialAro SpecialAro marked this pull request as ready for review May 11, 2024 01:34
@SpecialAro SpecialAro requested review from vraravam and a team May 11, 2024 01:36
@SpecialAro SpecialAro changed the title WIP: Fix notifications on all services Fix notifications on all services May 11, 2024
ponces pushed a commit to ponces/ferdium-app that referenced this pull request May 13, 2024
ponces pushed a commit to ponces/ferdium-app that referenced this pull request May 13, 2024
ponces pushed a commit to ponces/ferdium-app that referenced this pull request May 13, 2024
ponces pushed a commit to ponces/ferdium-app that referenced this pull request May 13, 2024
@SpecialAro SpecialAro merged commit 0ed2bac into ferdium:develop May 13, 2024
5 checks passed
@SpecialAro SpecialAro deleted the fix-notifications branch May 13, 2024 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants