Skip to content
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

Send a default action when notification is closed by user #3863

Conversation

BarbUk
Copy link
Contributor

@BarbUk BarbUk commented Oct 9, 2023

Really fix #3182

@codecov
Copy link

codecov bot commented Oct 9, 2023

Codecov Report

Merging #3863 (5daae2b) into master (aa8c7c6) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #3863   +/-   ##
=======================================
  Coverage   91.02%   91.03%           
=======================================
  Files         901      901           
  Lines       57537    57540    +3     
=======================================
+ Hits        52372    52380    +8     
+ Misses       5165     5160    -5     
Flag Coverage Δ
gcov 91.03% <100.00%> (+<0.01%) ⬆️
luacov 93.73% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
lib/naughty/dbus.lua 72.37% <100.00%> (+1.35%) ⬆️

... and 3 files with indirect coverage changes

@seqizz
Copy link

seqizz commented Oct 10, 2023

Thanks!

With this, looks like I can properly switching to the target channel, at least on on Slack & Telegram.

Yet I am not sure if we should include the feature (which is given here) directly into WM or leave it out.

@BarbUk
Copy link
Contributor Author

BarbUk commented Oct 10, 2023

Thanks!

With this, looks like I can properly switching to the target channel, at least on on Slack & Telegram.

I tested different notifications and applications from my side:

  • chrome / gmail (with two chrome instances running): it open the correct tab/mail
  • slack: open the correct channel
  • mattermost: open the correct channel

So it seems to work.

Yet I am not sure if we should include the feature (which is given here) directly into WM or leave it out.

It needs more work to handle edge case I think.
I could include it in another PR under a config flag like naughty.config.follow_clicked_notification.

In any case, it should be documented somewhere if people like to jump to the right client when clicking a notification.

Copy link
Member

@actionless actionless left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks 👌😺

@mergify mergify bot merged commit 7ed4dd6 into awesomeWM:master Oct 12, 2023
11 checks passed
@BarbUk BarbUk deleted the fix/notification_default_action_when_dismissed_by_user branch October 12, 2023 11:18
@BarbUk
Copy link
Contributor Author

BarbUk commented Oct 12, 2023

FYI this PR fix #3182 but it's not a perfect solution as it send a default action for all notification dismissed by a user.

So if I have a notification with two actions (default, action 1, action 2), and I click on the second action, it will send:

  • default
  • action 2

I'll make another PR with a proper fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clicking notification does not urgent/raise client
4 participants