Hi Sébastien, At 1252404617 time_t, Sébastien Gross wrote: > Tooltip can be added to any object having add_signal() method.
What a nice idea! :-)
> +-------------------------------------------------------------------------
> +-- Toolip module for Awesome
s/Awesome/awesome/
> +-- @author Sébastien Gross <seb•ɱɩɲʋʃ•awesome•ɑƬ•chezwam•ɖɵʈ•org>
> +-- @copyright 2009 Sébastien Gross
> +-- @release @AWESOME_VERSION@
> +-------------------------------------------------------------------------
> +
> +local mouse = mouse
> +local widget = widget
> +local screen = screen
> +local awful = require("awful")
> +local beautiful = require("beautiful")
> +local setmetatable = setmetatable
> +
> +module("awful.tooltip")
You need to set a useful description of your module and write proper
documentation how to use it. Refer to other (documented) module.
> +-- compute coords
> +local function set_geometry(obj)
This need cleaner documentation, as all function:
--- Compute coordinates.
-- @param obj The tooltip
> + local scr = mouse.screen
> + local s_geometry = screen[scr].workarea
> + local scr_w = s_geometry.x + s_geometry.width
> + local scr_h = s_geometry.y + s_geometry.height
> + local m_c = mouse.coords()
> + -- wibox size
> + local w_b = obj.tt_wibox.border_width
> + local w_h = obj.tt_wibox.height + 2 * w_b
> + local w_w = obj.tt_wibox.width + 2 * w_b
> +
> + local w = obj.tt_textbox:extents().width + obj.x_margin
> + local h = obj.tt_textbox:extents().height + obj.y_margin
> +
> + local x = m_c.x + obj.x_offset < s_geometry.x and s_geometry.x or m_c.x
> + obj.x_offset
> + local y = m_c.y + obj.y_offset < s_geometry.y and s_geometry.y or m_c.y
> + obj.y_offset
> +
> + y = y + w_h > scr_h and scr_h - w_h or y
> + x = x + w_w > scr_w and scr_w - w_w or x
> +
> + obj.tt_wibox:geometry({x = x, y = y, width = w, height = h})
> +end
That's a shame we don't have a lib with such functions. We already have
a placement.under_mouse for clients which does the same kind of things.
Well, not your fault anyway.
> +--- show
> +-- @param obj tooltip object
> +-- Show the tooltip
> +local function show(obj)
> + if obj.update_func ~= nil then
> + obj:set_text(obj.update_func())
> + end
> + set_geometry(obj)
> + obj.tt_wibox.visible = true
> +end
The update_func idea is nice, but might not be enough. I'd maybe rather
emit a signal when the tooltip is shown/hide, so the user can update the
text whenever it wants or do something else! :-)
> +--- hide
> +-- @param obj tooltip object
> +-- Hide the tooltip
> +local function hide(obj)
> + obj.tt_wibox.visible = false
> +end
As I will say later in this mail, you probably need to set/update
.screen also in this show and hide functions.
> +--- hide
> +-- @param obj tooltip object
> +-- @param text new tooltip text
> +-- Change the tooltip's text
> +local function set_text(obj, text)
> + obj.tt_textbox.text = '<span color="' .. obj.fg .. '" font_desc="'
> + .. obj.font .. '">' .. text .. "</span>"
> +end
> +
> +--- set_offset
> +-- @param obj tooltip object
> +-- @param x x offset
> +-- @param y y offset
> +-- Change the tooltip's offsets
> +local function set_offset(obj, x, y)
> + obj.x_offset = x
> + obj.y_offset = y
> +end
> +
> +--- set_margin
> +-- @param obj tooltip object
> +-- @param x x margin
> +-- @param y y margin
> +-- Change the tooltip's margins
> +local function set_margin(obj, x, y)
> + obj.x_margin = x
> + obj.y_margin = y
> +end
You define setters function. But, they are useless, because you just
expose the attribute of 'obj' by returning it to the user.
Since there's no need to define setters, I'd remove them and document
the 'obj' attributes in luadoc so the user know which field are used by
the code. :-)
> +local function set_defaults(obj)
> + obj.tt_wibox.border_width = beautiful.tooltip_border_width or
> beautiful.border_width or 1
> + obj.tt_wibox.border_color = beautiful.tooltip_border_color or
> beautiful.border_normal
> + obj.tt_wibox.opacity = beautiful.tooltip_opacity or 1
> + obj.tt_wibox.bg = beautiful.tooltip_bg_color or beautiful.bg_focus or
> "#ffcb60"
> + obj.fg = beautiful.tooltip_fg_color or beautiful.fg_focus or "#000000"
> + obj.font = beautiful.tooltip_font or beautiful.font or "terminus 6"
> +end
> +
> +
> +--- create
> +-- @param _
> +-- @param on_widget the widget to be linked with the tooltip
> +-- @param func a function to dynamicaly change the tooltip text
> +-- Create a new tooltip and link it to a widget
> +local function create(_, on_widget, func)
> + local obj = {
> + x_offset = 1,
> + y_offset = 1,
> + x_margin = 2,
> + y_margin = 2,
> +
> + tt_wibox = awful.wibox({
> + position = "float",
> + }),
Err, position = "float" is not correct. You probably need to use wibox()
directly. awful.wibox() creates wibox attached to the border of the
screen, it's not meant to be used for floating wiboxes.
Besides that, you never set the screen of the wibox, you let awful.wibox
set it to 1, which will fails in multi-head env.
> + tt_textbox = widget({type = "textbox", name = "tt",
> align="right"}),
> + update_func = func
> + }
> + set_defaults(obj)
> + obj.tt_wibox.visible = false
> + obj.tt_wibox.ontop = true
> + obj.tt_wibox.widgets = { obj.tt_textbox }
> + obj.show = function(obj) show(obj) end
> + obj.hide = function(obj) hide(obj) end
> + obj.set_text = function(obj, t) set_text(obj, t) end
> + obj.set_offset = function(obj, x, y) set_offset(obj, x, y) end
> + obj.set_margin = function(obj, x, y) set_margin(obj, x, y) end
You're creating unnecessary closure here.
What you have to do is that:
obj.set_text = set_text
This is enough, the user will just have to call:
obj:set_text("bla") (note the ':')
which will actually do: obj.set_text(obj, "bla")
and avoid useless closures.
> +setmetatable(_M, { __call = create })
Please, do as we do usually:
__call = function(_, ...) return new(...) end
And rename create to new so we are consitent with the rest of the
modules.
Cheers,
--
Julien Danjou
// ᐰ <[email protected]> http://julien.danjou.info
// 9A0D 5FD9 EB42 22F6 8974 C95C A462 B51E C2FE E5CD
// Life is life. Lalalalala.
signature.asc
Description: Digital signature
