From e524f93baa05db2233b47adc624b8b5671f1aa0a Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Wed, 17 Jul 2019 00:26:44 -0400 Subject: [PATCH] notification.action: Allow actions to be shared by multiple notification The reason is that if actions are provided by rules, only one instance exist. It was a mistake to couple actions with their notifications. It could not work reliably and has to be removed. The commit also change the notification action storage to be a copy instead of the original table. This allows to append actions (not part of this commit) without risking adding them to the wrong notification. **WARNING** This break an unreleased API by removing the `notification` property of an action. --- lib/naughty/action.lua | 56 +++++++++++++------------------- lib/naughty/layout/legacy.lua | 6 ++-- lib/naughty/list/actions.lua | 12 +++---- lib/naughty/notification.lua | 61 +++++++++++++++++++++++++++++------ 4 files changed, 81 insertions(+), 54 deletions(-) diff --git a/lib/naughty/action.lua b/lib/naughty/action.lua index dcd234096..3ed71114e 100644 --- a/lib/naughty/action.lua +++ b/lib/naughty/action.lua @@ -2,7 +2,8 @@ --- A notification action. -- -- A notification can have multiple actions to chose from. This module allows --- to manage such actions. +-- to manage such actions. An action object can be shared by multiple +-- notifications. -- -- @author Emmanuel Lepage Vallee <elv1313@gmail.com> -- @copyright 2019 Emmanuel Lepage Vallee @@ -29,8 +30,8 @@ local action = {} -- If the action is selected. -- --- Only a single action can be selected per notification. It will be applied --- when `my_notification:apply()` is called. +-- Only a single action can be selected per notification. This is useful to +-- implement keyboard navigation. -- -- @property selected -- @param boolean @@ -50,10 +51,6 @@ local action = {} -- @property icon_only -- @param[opt=false] boolean ---- The notification. --- @property notification --- @tparam naughty.notification notification - --- When a notification is invoked. -- @signal invoked @@ -64,10 +61,7 @@ end function action:set_selected(value) self._private.selected = value self:emit_signal("property::selected", value) - - if self._private.notification then - self._private.notification:emit_signal("property::actions") - end + self:emit_signal("_changed") --TODO deselect other actions from the same notification end @@ -79,15 +73,12 @@ end function action:set_position(value) self._private.position = value self:emit_signal("property::position", value) - - if self._private.notification then - self._private.notification:emit_signal("property::actions") - end + self:emit_signal("_changed") --TODO make sure the position is unique end -for _, prop in ipairs { "name", "icon", "notification", "icon_only" } do +for _, prop in ipairs { "name", "icon", "icon_only" } do action["get_"..prop] = function(self) return self._private[prop] end @@ -95,22 +86,18 @@ for _, prop in ipairs { "name", "icon", "notification", "icon_only" } do action["set_"..prop] = function(self, value) self._private[prop] = value self:emit_signal("property::"..prop, value) - - -- Make sure widgets with as an actionlist is updated. - if self._private.notification then - self._private.notification:emit_signal("property::actions") - end + self:emit_signal("_changed") end end -local set_notif = action.set_notification - -function action.set_notification(self, value) - local old = self._private.notification - set_notif(self, value) - if old then - old:emit_signal("property::actions") - end +--TODO v4.5, remove this. +function action.set_notification() + -- It didn't work because it prevented actions defined in the rules to be + -- in multiple notifications at once. + assert( + false, + "Setting a notification object was a bad idea and is now forbidden" + ) end --- Execute this action. @@ -118,11 +105,12 @@ end -- This only emits the `invoked` signal. -- -- @method invoke -function action:invoke() - assert(self._private.notification, - "Cannot invoke an action without a notification") - - self:emit_signal("invoked") +-- @tparam[opt={}] naughty.notification notif A notification object on which +-- the action was invoked. If a notification is shared by many object (like +-- a "mute" or "snooze" action added to all notification), calling `:invoke()` +-- without adding the `notif` context will cause unexpected results. +function action:invoke(notif) + self:emit_signal("invoked", notif) end local function new(_, args) diff --git a/lib/naughty/layout/legacy.lua b/lib/naughty/layout/legacy.lua index 21e97444d..ee99c0837 100644 --- a/lib/naughty/layout/legacy.lua +++ b/lib/naughty/layout/legacy.lua @@ -430,12 +430,10 @@ function naughty.default_notification_handler(notification, args) actionmarginbox:buttons(gtable.join( button({ }, 1, function() - action:invoke() - notification:destroy() + action:invoke(notification) end), button({ }, 3, function() - action:invoke() - notification:destroy() + action:invoke(notification) end) )) actionslayout:add(actionmarginbox) diff --git a/lib/naughty/list/actions.lua b/lib/naughty/list/actions.lua index cc37d703c..ff9c8e5eb 100644 --- a/lib/naughty/list/actions.lua +++ b/lib/naughty/list/actions.lua @@ -117,10 +117,6 @@ local module = {} -- @tparam gears.surface|string action_bgimage_selected -- @see gears.surface -local default_buttons = gtable.join( - abutton({ }, 1, function(a) a:invoke() end) -) - local props = {"shape_border_color", "bg_image" , "fg", "shape_border_width", "underline", "bg", "shape", "icon_size", } @@ -176,7 +172,7 @@ local function update(self) awcommon.list_update( self._private.layout, - default_buttons, + self._private.default_buttons, function(o) return wb_label(o, self) end, self._private.data, self._private.notification.actions, @@ -273,7 +269,7 @@ end --- Create an action list. -- -- @tparam table args --- @tparam naughty.notification args.notification The notification/ +-- @tparam naughty.notification args.notification The notification. -- @tparam widget args.base_layout The action layout. -- @tparam table args.style Override the beautiful values. -- @tparam boolean args.style.underline_normal @@ -312,6 +308,10 @@ local function new(_, args) update_style(wdg) + wdg._private.default_buttons = gtable.join( + abutton({ }, 1, function(a) a:invoke(args.notification) end) + ) + return wdg end diff --git a/lib/naughty/notification.lua b/lib/naughty/notification.lua index 72ecab225..b1a89b08c 100644 --- a/lib/naughty/notification.lua +++ b/lib/naughty/notification.lua @@ -641,6 +641,34 @@ function notification.get_clients(self) return self._private.clients end +function notification.set_actions(self, new_actions) + for _, a in ipairs(self._private.actions or {}) do + a:disconnect_signal("_changed", self._private.action_cb ) + a:disconnect_signal("invoked" , self._private.invoked_cb) + end + + -- Clone so `append_actions` doesn't add unwanted actions to other + -- notifications. + self._private.actions = gtable.clone(new_actions, false) + + for _, a in ipairs(self._private.actions or {}) do + a:connect_signal("_changed", self._private.action_cb ) + a:connect_signal("invoked" , self._private.invoked_cb) + end + + self:emit_signal("property::actions", new_actions) + + -- When a notification is updated over dbus or by setting a property, + -- it is usually convenient to reset the timeout. + local reset = ((not self.suspended) + and self.auto_reset_timeout ~= false + and naughty.auto_reset_timeout) + + if reset then + self:reset_timeout() + end +end + --TODO v6: remove this local function convert_actions(actions) gdebug.deprecate( @@ -808,21 +836,34 @@ local function create(args) private[k] = v end - -- notif.actions should not be nil to allow cheching if there is actions - -- using the shorthand `if #notif.actions > 0 then` - private.actions = private.actions or {} - - -- Make sure the action are for this notification. Sharing actions with - -- multiple notification is not supported. - for _, a in ipairs(private.actions) do - a.notification = n - end - -- It's an automatic property n.is_expired = false gtable.crush(n, notification, true) + -- Always emit property::actions when any of the action change to allow + -- some widgets to be updated without over complicated built-in tracking + -- of all options. + function n._private.action_cb() n:emit_signal("property::actions") end + + -- Listen to action press and destroy non-resident notifications. + function n._private.invoked_cb(a, notif) + if (not notif) or notif == n then + n:emit_signal("invoked", a) + + if not n.resident then + n:destroy(cst.notification_closed_reason.dismissed_by_user) + end + end + end + + -- notif.actions should not be nil to allow checking if there is actions + -- using the shorthand `if #notif.actions > 0 then` + private.actions = {} + if args.actions then + notification.set_actions(n, args.actions) + end + n.id = n.id or notification._gen_next_id() -- Register the notification before requesting a widget