Electron provides easy way to set the badge. This diff sets it to the unread count whenever it changes.
Details
Details
- Reviewers
tomek atul • abosh kamil - Commits
- rCOMM40c766e4e726: [desktop] Notification badge
- Send a message from another account and check if the badge is displayed
- Read the message and check if the badge disappears
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
I think it would be good for @kamil to review this too – he's looking at ENG-1687: Explore sqlite-worker NPM package, which is related to ENG-1685: Stateful web client, which I think has some connection to the Electron app. Given this overlap I think it makes sense for @michal to review @kamil's diffs in the space, and for @kamil to review @michal's diffs in the space.
| desktop/main.js | ||
|---|---|---|
| 74–78 ↗ | (On Diff #18426) | |
| web/app.react.js | ||
| 264–267 ↗ | (On Diff #18426) | I've noticed that this code doesn't seem to run when the app is in a tab that isn't the main tab. Not sure how that applies to Electron (maybe it isn't relevant). Either way, we should probably have a task to resolve the issue on web |
| desktop/main.js | ||
|---|---|---|
| 73 ↗ | (On Diff #18426) | Can we annotate the types of parameters? |
| desktop/main.js | ||
|---|---|---|
| 73 ↗ | (On Diff #18426) | I have explained more about the typing in D5629. |
| web/app.react.js | ||
| 264–267 ↗ | (On Diff #18426) | Yeah, it seems that the electron app isn't updated when the window isn't in the foreground. I think it's a general update from the server that doesn't run, not just this code right? We should figure out a way to update when the web/ desktop app is in the background (this also blocks notification work, because they won't really be useful otherwise). |
| web/app.react.js | ||
|---|---|---|
| 264–267 ↗ | (On Diff #18426) | Can you please create a task and link here? |
Comment Actions
Would be helpful in future for this sort of visual thing to include a screenshot in the Summary or Test Plan... especially since this one is probably a little harder to reproduce in reviewer's local dev environment?