-
-
Notifications
You must be signed in to change notification settings - Fork 565
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: allow to set mute duration and notifications mute option #2665
Conversation
Run & review this pull request in StackBlitz Codeflow. |
✅ Deploy Preview for elk-docs canceled.
|
✅ Deploy Preview for elk-zone ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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 great! A step closer to feature parity 🙌🏼
I think we should merge this one now, and later on we could iterate and refactor it if we end up with more extra options. It is probably better that the confirm dialog will have a slot where you could inject extra options, and have each of these dialogs as a separate component.
@patak-dev Thanks for a good suggestion! Right, it was a better choice to use slot inject extra options, instead of adding up pieces only needed for the mute functionality in this case. I think I can refactor it later. Since I actually haven't had so much experience composing a component using the slot, that'll be a good exercise for me too 🙂 |
resolve #2301
This PR adds the two mute options in the confirm dialog for muting.
I set the default duration setting as 1 hour (this can be changed if the other value is better), and the mute notifications enabled, which is the current default.
Screenshots
Default
Setting example
Invalid duration
disables the "Mute" button
Implementation notes
To implement this feature, I adjusted the existing
ConfirmDialog
component to be able to show mute-related options and return selected information. (4276b50 cca244a)Also, I implemented the
DurationPicker
component (ee0742e), where we can select minutes with 5 minutes step. I chose a 5-minute step for the minutes input to make it easier to select, but users can input any minute value type by directly typing.Another adjustment is
CommonCheckbox
(bbb15dc). Previously, it could only show its checkbox at the end of the right side like on the settings page. This change allows us to put the checkbox before the label and accept the checkbox color change too.