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

feat: show system notifications #510

Open
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

Misieq01
Copy link
Collaborator

@Misieq01 Misieq01 commented Sep 18, 2024

[ New Feature ] => Notifications

Implemented reusable method for triggering display of system notifications with desired title and body message

Platform specifics

Linux:

  • Working on both dev and production environments
  • Missing Tari icon for notification

Windows

  • Working on both dev and production environments
  • Notifications displayed as if they have been send from powershell

MacOS

  • Working only on production environment
  • To make it work user have to enable notifications for Tari app and go to notifications center and switch notficiation display from banner to alert for Tari

[ @tauri-apps/plugin-notification ]

Currently implemented and replaced old implementation with official tauri plugin
After testing it looks like it work similar as my implementation or even slightly worse

Platform specifics

Linux:

  • Working only on production environment ( build locally )
  • Missing Tari icon for notification

Windows

  • Working on both dev and production environments
  • Notifications ON DEV displayed as if they have been send from powershell
  • Notifications ON BUILD displayed with proper tari icon

MacOS

  • Working only on production environment
  • To make it work user have to enable notifications for Tari app and go to notifications center and switch notficiation display from banner to alert for Tari

@Misieq01 Misieq01 force-pushed the feat/show-system-notifications branch 2 times, most recently from 3b70dcb to d5216e5 Compare September 20, 2024 13:41
@Misieq01 Misieq01 force-pushed the feat/show-system-notifications branch from d5216e5 to a9b9a03 Compare September 24, 2024 07:07
Copy link
Collaborator

@brianp brianp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good. It's at least the second time we're using this pattern so it could be good to move the repeated bits to a trait if possible.

Once the branch is updated I'll test.

src-tauri/src/notification_manager.rs Outdated Show resolved Hide resolved
src-tauri/src/notification_manager.rs Outdated Show resolved Hide resolved
src-tauri/src/notification_manager.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@brianp brianp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏🏻 Excellent stuff

Copy link
Collaborator

@brianp brianp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried running in both dev, and with a local build. MacOS doesn't seem to know what to do with the notifications. And prompts with a confusing message Saying to Choose an Application and "Where is use_default".

image

src-tauri/src/notification_manager.rs Outdated Show resolved Hide resolved
@Misieq01
Copy link
Collaborator Author

I tried running in both dev, and with a local build. MacOS doesn't seem to know what to do with the notifications. And prompts with a confusing message Saying to Choose an Application and "Where is use_default".

image

This feature does not work on macos in dev mode. Did it happen also in local build version ?

@brianp
Copy link
Collaborator

brianp commented Oct 28, 2024

Sorry I didn't re-read the notes after the weekend and forgot it was prod mode only.

We should disable this in the dev environment. It's quite a large nuisance if it can't be used, as it pops up those dialogs on every block win.

When notifications aren't enabled yet we still receive the dialog to Choose Application even on the prod build.
Once enabled we need to format the win value correctly:
image

@Misieq01
Copy link
Collaborator Author

Sorry I didn't re-read the notes after the weekend and forgot it was prod mode only.

We should disable this in the dev environment. It's quite a large nuisance if it can't be used, as it pops up those dialogs on every block win.

When notifications aren't enabled yet we still receive the dialog to Choose Application even on the prod build. Once enabled we need to format the win value correctly: image

I should be able to disable triggering of showing notifications on dev environment and also format the amounts, but as for the dialog on macos for choosing application I'm not sure if I will be able to do something with it

@brianp
Copy link
Collaborator

brianp commented Oct 28, 2024

I should be able to disable triggering of showing notifications on dev environment and also format the amounts, but as for the dialog on macos for choosing application I'm not sure if I will be able to do something with it

It seems strange. It's a very terrible UX experience. But this isn't normal. Shouldn't we be able to prompt for notifications permission or something similar instead of trying to use notifications, and showing the Choose Application dialog. I feel like there's a step missing somewhere.

@Misieq01
Copy link
Collaborator Author

I should be able to disable triggering of showing notifications on dev environment and also format the amounts, but as for the dialog on macos for choosing application I'm not sure if I will be able to do something with it

It seems strange. It's a very terrible UX experience. But this isn't normal. Shouldn't we be able to prompt for notifications permission or something similar instead of trying to use notifications, and showing the Choose Application dialog. I feel like there's a step missing somewhere.

Its old Pr, I can go back and review how I've implemented this and if there is some other option but as far as I remember this was best that I could made in decent amount of time that would work more or less

@brianp
Copy link
Collaborator

brianp commented Oct 28, 2024

It may be useful to use Tauri's existing plugin for system notifications: https://github.com/tauri-apps/tauri-plugin-notification

It should allow us to perform actions like ensuring we have permission before sending. It also appears to handle the OS dependent sending switches.

@Misieq01
Copy link
Collaborator Author

It may be useful to use Tauri's existing plugin for system notifications: https://github.com/tauri-apps/tauri-plugin-notification

It should allow us to perform actions like ensuring we have permission before sending. It also appears to handle the OS dependent sending switches.

That was first thing that I tried but it didn't work at all. At least with my setup

@brianp brianp added the on hold Issue is being held for progress later label Oct 31, 2024
@brianp
Copy link
Collaborator

brianp commented Oct 31, 2024

on hold until the tauri v2 upgrade is done, as we suspect the API will become easier to deal with.

@brianp
Copy link
Collaborator

brianp commented Dec 4, 2024

Moving this into the Todo list. Once V2 is merged let's revisit the tauri notifications plugin, and see if we can get rid of the weird "Choose application" dialog in mac os.

@brianp brianp removed the on hold Issue is being held for progress later label Dec 4, 2024
@Misieq01 Misieq01 force-pushed the feat/show-system-notifications branch from 7ba3886 to 18abb60 Compare December 12, 2024 07:59
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