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.
This commit is contained in:
Emmanuel Lepage Vallee 2019-07-17 00:26:44 -04:00
parent f8cbb54913
commit e524f93baa
4 changed files with 81 additions and 54 deletions

View File

@ -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)

View File

@ -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)

View File

@ -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

View File

@ -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