Keyboard event-related fixes + notifications improvements

[Oaphi]

closes #1824
completes meta:293809 (hasn't been able to find an issue that corresponds to it)

Also finally moves our keyboard shortcuts manager from a global _codidactKeyboard variable to the QPixel namespace (Keyboard), as well as simplifies its initialization handling.

+ fixes the inbox icon doing nothing for users with JavaScript disabled (it's now linked to the full inbox page);
+ allows inbox notifications (on the full page) to be filtered by status (all / read / unread), as well as ordered by "oldest first";
+ adds an icon indicating whether the notification is read or not (we don't have any currently):

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.10%. Comparing base (c6816f9) to head (2c3433e).

Additional details and impacted files

Components	Coverage Δ
controllers	73.24% <100.00%> (+0.09%)	⬆️
helpers	84.16% <ø> (-0.51%)	⬇️
jobs	60.86% <ø> (ø)
models	89.27% <100.00%> (+0.02%)	⬆️
tasks	61.11% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[cellio]

Keyboard: I'm not seeing a behavior change. On a Mac, cmd-click launches a new tab and closes the notifications, and ctrl-click gives me the right-click menu (normal behavior). I restarted the server before testing and I don't see any db changes here. Is there something else I need to do to activate this?

Red outline: confirmed working in Firefox. (Bug is apparently masked in Chrome.)

[trichoplax]

I'm not available to test at the moment (I never have access to a Mac setup and I'm currently away from my Linux setup), so these questions are based solely on guesswork:

Possible other fix

Does this also fix Stuck red border after reducing below maximum characters (asking because it is so closely related to the linked meta post, so might be worth testing)? Not a request to include it if not. Also I'm not sure if I understood a recent conversation that suggested one or other of them was already fixed or due to be, so this question might turn out to be irrelevant.

Possible unintended Windows behaviour

From a brief glance at the code, could it be that the code:

if (ev.ctrlKey || ev.metaKey) {

causes the shortcuts that previously worked on Windows/Linux with the ctrl key to now also work on Windows/Linux with the Windows key? Regardless of whether it's working correctly on a Mac, I'm guessing we don't want to override Windows key shortcuts for Windows/Linux users? Since I can't currently test I don't know if this is the case, but it seems like something we should check carefully on both those operating systems before proceeding.

I don't know the correct way to handle this, but I'm guessing we either need to detect operating system rather than just detect either modifier button (so we only respond to one specific modifier button per operating system) or we need some way that the browser can do the interpretation for us (not hopeful about that and haven't looked into it).

[Oaphi]

we either need to detect operating system

We can't, there is no reliable way of determining whether the user is on MacOS or not from JS (thankfully so), the best option is to use two keys that are specified to work this way.

Does this also fix Stuck red border after reducing below maximum characters

Yup, it does (at least it should), and, IIRC, @cellio confirmed it should be working in this PR

[Oaphi]

Is there something else I need to do to activate this?

No, @cellio, it should work out of the box - I'll take a look

[cellio]

Does this also fix Stuck red border after reducing below maximum characters (asking because it is so closely related to the linked meta post, so might be worth testing)?

Yes. I tested both linked meta cases on this branch in Firefox and saw red borders where they're supposed to be and not otherwise.

[cellio]

I pulled this branch again (to test the red-outline thing for comments), and now I'm seeing cmd-click open a notification in a separate tab (Firefox and Chrome). It also marks the notification as read, but right-clicking to open in a new tab does not (never did). Which behavior do users of cmd-click/ctrl-click expect?

I don't have a Windows machine to test ctrl-click on.

[trichoplax]

Which behavior do users of cmd-click/ctrl-click expect?

I don't know which behaviour is expected, but the inconsistencies are probably best dealt with in a separate issue since they go beyond what is being changed here.

It also marks the notification as read, but right-clicking to open in a new tab does not (never did).

In addition to things like this not being picked up by the JavaScript code for users who have JavaScript, users with JavaScript disabled get no response when clicking on the new notification circle or the envelope icon, so they have no way to view the notifications unless they know the URL for the stand alone notifications page, and even once there they cannot mark notifications as read.

Maybe things that don't yet result in a notification being marked as read could be added to a separate issue to make notifications available to users with JavaScript disabled, along with a way to mark notifications as read without JavaScript?

[trichoplax]

I've added #1833. Feel free to add subtasks to that one that don't need to be addressed here.

[cellio]

Nice improvements! cmd-click now opens an inbox item in a new tab and marks it as read (tested in Chrome), and with JS disabled the notification icon takes me to the inbox page (tested in Firefox). The new filter buttons are great, too!

I am confused by the icons on the inbox page. In the notifications drop-down, I always assumed that the icon corresponded to the action in the link -- "mark read" = open the envelope, "mark unread" = close that thing back up in its envelope for later processing. But now we're using an icon tied to an action to report a status, and it feels like maybe they're backwards? An item with the opened envelope is unread on the inbox page:

It becomes clear enough what the icons mean if you use the buttons, but I wonder if we should reverse them, and/or add the icons into the buttons too, so you'd have the "key" right there when viewing the page, like this:

Note: if these icons on the full inbox page were controls, i.e. clicking them changed the status, then I'd agree that we'd want the icons as they are. It would be good to add controls, which we do not need to do here. So maybe, having written all this, I've talked myself into having no icons now, neither the lines nor the buttons, and adding icons when they are controls like in the notifications pane. I'd definitely like to hear other thoughts here.

[Oaphi]

I am confused by the icons on the inbox page. In the notifications drop-down, I always assumed that the icon corresponded to the action in the link -- "mark read" = open the envelope, "mark unread" = close that thing back up in its envelope for later processing.

To be honest, those icons are there only because I failed to find a better way of separating read notifications from unread - suggestions welcome. Adding icons to the filter buttons sounds good to me, but let's see if someone can come up with a better approach

[cellio]

To be honest, those icons are there only because I failed to find a better way of separating read notifications from unread - suggestions welcome. Adding icons to the filter buttons sounds good to me, but let's see if someone can come up with a better approach

Is the light blue background, like in the notifications pane, possible? And then that saves the icon "slot" for a control later.