-
-
Notifications
You must be signed in to change notification settings - Fork 473
Feature/dnd duration dropdown #1419
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
base: main
Are you sure you want to change the base?
Conversation
|
||
.dropdown div:hover { | ||
background-color: rgb(68 68 68 / 100%); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not well-scoped CSS; you don't want to apply CSS to entire rules like div
for readability and fragility reasons.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handled this by applying style to specific class.
app/renderer/js/main.ts
Outdated
@@ -1210,6 +1231,13 @@ window.addEventListener("load", async () => { | |||
>${t.__("Do Not Disturb")}</span | |||
> | |||
</div> | |||
<div id="dnd-dropdown" class="dropdown hidden"> | |||
<div data-minutes="1">1 minute</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 minute seems basically useless in practice -- is that just for testing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it was for testing. I have removed it in the commits.
@lolostheman thanks for the PR! Can you clean up the commit history to make it easier to review? The commits here seem to have a lot of churn where one commit is fixing up issues in the previous one. Especially code moves are best done in commits that don't make any changes other than moving the code and adjusting imports. Check out our GitHub guide and commit guidelines for advice on how to do a nicely reviewable PR. |
If the app exits (or the computer crashes, etc.) before the timer expires, this will make the temporary setting permanent. I don’t think that’s what a user would expect. |
…s add new toggle-dnd-request RenderMessage. Introduces an optional duration parameter to the toggle-dnd-request message, enabling the renderer to specify a time value for automatically reverting Do Not Disturb (DND) mode. The toggle-dnd-request event is now used to send this duration from the client to the main process. The toggle-dnd response has been updated to include the duration, allowing the renderer to reflect the timeout state accurately.
9268d5f
to
684da87
Compare
Yes, this would indeed happen. if the app crashes before the setTimeout callback fires. I'll work on a fix. Probably by storing the duration in the config file, and then we can check if the duration has expired on startup. |
Introduces a hidden dropdown menu with preset duration options and accompanying styles. This prepares the UI for timed DND selection.
Implements click and mouseleave behavior for the DND button and duration dropdown, allowing the user to select a duration visually. We also remove the dnd-util.js import, since we no longer handle the DNDtoggle() here and instead we send the toggle request to the ipcMain.
When a duration is provided, sets a timer to automatically revert DND mode. Ensures only one timer is active at a time, and cleans up appropriately when DND is turned off early. We import dnd-util.js to handle the DNDUtil.toggle() in our timer logic. Fixes zulip#561.
Checks the dndExpiration config key on app startup. If the timestamp has passed, DND is toggled off. If still active, schedules a timeout to revert DND later.
684da87
to
dd34bbd
Compare
@lolostheman Is this PR ready for the next round of review? It's helpful to post a comment explicitly noting this if so. |
@alya Yes, the PR is ready for another review. |
Added drop down menu for users to enable Do Not Disturb (DND) mode for a selectable duration, after which the app will automatically restore the original notification settings.
First had the logic in the dnd-util.ts. Got this working, but wanted to handle the dnd-toggle request logic in app/main/index.ts to better follow Zulip's contribuiting guide.
We listen for our LeftSidebarEvents() & dndButton EventListener() aka our renderer to send a message to our ipcMain.on() in app/main/index.ts. This is where we now handle the logic and send message back to the renderer with the new parameters.
Had some small UI issues, with modal overlays covering things. Was able to fix. Tested functionality with numerous test cases, logging results to console. Haven't found an edge case that breaks the code.
Feel free to comment on style or content within the dropdown.
This is my first open source contribution. Please bare with me. Thanks!
Fixes: (#561)
Tooling tips: https://zulip.readthedocs.io/en/latest/tutorials/screenshot-and-gif-software.html
-->
Screenshots and screen captures:


Platforms this PR was tested on:
Self-review checklist
(variable names, code reuse, readability, etc.).
Communicate decisions, questions, and potential concerns.
Individual commits are ready for review (see commit discipline).
Completed manual review and testing of the following: