Rich notifications

[Novack]

Reworked notifications by replacing custom package by native windows notifications. Added rich notifications with custom actions; these actions work on Windows 10 only, so added a simple notification fallback for prior versions.

[Novack]

Note that there are currently 5 notification actions available:

• openThread, (will open the window with the selected thread opened)
• markAsRead,
• markAsSpam (currently hidden),
• sendToTrash,
• archive,

All of then send the thread id, using the data model previously discussed. Let me know if any info is needed.

[amilich]

This is awesome :)

[Novack]

Javascript is not my area, but I understand is something like:

window.chrome.webview.addEventListener('message', event => {
    // Handle the message here
    console.log(event.data);
});

Then your parse event.data based on the model defined in the classes at the bottom of MessageProcessor class in the desktop app project.

[amilich]

Javascript is not my area, but I understand is something like:

window.chrome.webview.addEventListener('message', event => {
    // Handle the message here
    console.log(event.data);
});

Then your parse event.data based on the model defined in the classes at the bottom of MessageProcessor class in the desktop app project.

Awesome. Will try it out.

[Novack]

Poke me if you need any change or a hand with this one @amilich.

[amilich]

Poke me if you need any change or a hand with this one @amilich.

Will do :)

I need to get the web code side done!

[amilich]

What is the JSON structure?

            var notificationData = new NotificationActionData()
            {
                Action = action,
                ThreadId = threadId,
            };

            var messageWrapper = new MessageWrapper()
            {
                MsgType = MessageTypes.notificationAction,
                Data = notificationData
            };

{
 msgType: 'notificationAction'
 data: {
    action: 'archive',
    threadID: string
 }
}

[Novack]

Yes exactly that (the JsonSerializer uses \") "{ \"type\":\"notificationAction\", \"data\": { \"Action\":\"markAsRead\", \"ThreadId\":\"f395cec4-f040-490a-9465-481be7d62f9b\" } }"

[amilich]

hopefully will get this out today

[amilich]

might need to tweak capitalization

[Novack]

might need to tweak capitalization

Was about to mention that threadID and threadId need consolidation :) Also the fields name PascalCasing vs camelCasing.

[amilich]

Action

code is ready - confirming:

const parsedData = JSON.parse(event.data) as { type: string; data: { Action: string; threadId: string } };

capitalization correct?

[Novack]

capitalization correct?

Both Action and ThreadId are PascalCase in C# side (because I used the default C# field names as json output), but I missed that the json is actually camelCased in all cases, so let me fix that for consistency.

End result will be action and threadId. So in your example, only "Action" needs to be changed to "action".

[amilich]

capitalization correct?

Both Action and ThreadId are PascalCase in C# side (because I used the default C# field names as json output), but I missed that the json is actually camelCased in all cases, so let me fix that for consistency.

End result will be action and threadId. So in your example, only "Action" needs to be changed to "action".

Making this change. Will be released to production in ~2 hours.

action and threadId

[amilich]

@Novack released in production!

[Novack]

@Novack released in production!

None of the actions seem to be working, I'll review it later tonight.

[amilich]

@Novack released in production!

None of the actions seem to be working, I'll review it later tonight.

oh no :(

here's the code:

 useEffect(() => {
    if (!isWindowsDesktopApp()) {
      return;
    }
    const notificationActionListener = (event: MessageEvent) => {
      // actions available are `openThread`, `markAsRead`, `markAsSpam`, `sendToTrash`, `archive`
      try {
        // eslint-disable-next-line @typescript-eslint/no-unsafe-argument
        const parsedData = JSON.parse(event.data) as { type: string; data: { Action: string; threadId: string } };
        if (parsedData.type === 'notificationAction') {
          const { Action: action, threadId: threadID } = parsedData.data;
          if (action === 'openThread') {
            setActiveThreadID({ threadID });
          } else if (action === 'markAsRead') {
            void markThreadsAsRead([threadID]);
          } else if (action === 'markAsSpam') {
            void moveThreads([threadID], LABEL_TO_SYSTEM_LABEL[SystemLabels.Spam], [label]);
          } else if (action === 'sendToTrash') {
            void trashThreads([threadID], false);
          } else if (action === 'archive') {
            void archiveThreads([threadID]);
          }
        }
      } catch (err) {
        console.error('Failed to parse data for Windows action', err);
      }
    };
    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
    // @ts-ignore
    // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-call
    chrome.webview.addEventListener('message', notificationActionListener);
    return () => {
      // eslint-disable-next-line @typescript-eslint/ban-ts-comment
      // @ts-ignore
      // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-call
      chrome.webview.removeEventListener('message', notificationActionListener);
    };
  }, []);

[amilich]

actually maybe action is misspelled here. I might have missed the last push.

[Novack]

Yep, seems like Action should be action, and in some places you're using threadID instead of threadId, though not sure if you're referencing ther local value! Will have a detained look in a few minutes.

[amilich]

const { Action: action, threadId: threadID } = parsedData.data;

I just renamed theadId to be threadID here! That should be fine.

[Novack]

Ok just confirmed that if I rename from "action" to "Action" in the desktop project, all actions work. But for consistency would be better to change that on javascript, what do you think?

Also noted that if you're minimized, but in other folder than Inbox, newMessageNotifications message never arrives. On the other side (as we know already) unreadMailCount changes from folder to folder. So we need the opposite:

• newMessageNotifications to work regardless of what folder the user is at.
• unreadMailCount to only work for inbox.

[amilich]

Should be fixed in JS as of ~2 minutes ago

[amilich]

I see

[amilich]

feel free to merge!

[Novack]

feel free to merge!

Thank you! I dont think I have write permissions, lol You need to merge :)

[amilich]

also they seem default inside the notifications center, not banner

[Novack]

@amilich this may be related to some setting in Windows perhaps? I am on Windows 10, and the notifications look as I have shown on the op. I'll google a bit to see if I am missing some detail on the API.

[Novack]

I think something in your build or session failed, even in the notification center, I can see the new rich notifications (1), but not for those received by the latest public release which does not include rich notifications (2).

[amilich]

I think something in your build or session failed, even in the notification center, I can see the new rich notifications (1), but not for those received by the latest public release which does not include rich notifications (2).

I agree, I will test tonight more.

[amilich]

@Novack IT WORKED! Just rebuilt. Sorry for the delay. Building production shortly.

Was doing some work on Quick Aliases until Monday.

[Novack]

Awesome news! Thanks for looking into it. Great work on the Quick Aliases!! 👏👏👏

[amilich]

Thank you!

Build is here Skiff Installer.msi.zip

[amilich]

My installation looks good! Going to make a release now.