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

disconnecting signals #71

Open
basaran opened this issue Jul 3, 2021 · 3 comments
Open

disconnecting signals #71

basaran opened this issue Jul 3, 2021 · 3 comments
Labels
bug Something isn't working

Comments

@basaran
Copy link

basaran commented Jul 3, 2021

Hello,

I have been "studying" your tab design (with titlebars) to incorporate the logic into my own module, and there is one thing that puzzles me:

bling/widget/tabbar/default.lua

 c:connect_signal("property::name", function(_)
        local title_temp = c.name or c.class or "-"
        text_temp.markup = "<span foreground='" .. fg_temp .. "'>" .. title_temp.. "</span>"
end)

This signal appears to be running in a loop for each titlebar tab getting created. Wouldn't it create a lot of duplicate signals over the same client? The situation appears to be getting worse if you detach and reattach the same client to the tab group.

I tried to go around it by assigning some if checks but that takes away the functionality of updating the tabs with the changed titles. I tried adding an id field to be used with get_children_by_id but I haven't succeeded on that yet. This get_children_by_id is a bit twilight zone to me.

    local wid_temp = wibox.widget({
        id = c.window .. idx
        text_temp,
        buttons = buttons,
        bg = bg_temp,
        widget = wibox.container.background()
    })

idx was basically something I passed along to the create during iteration. It appeared to work, but I couldn't access anything.

Any thoughts?

@javacafe01
Copy link
Member

@Nooo37

@Nooo37
Copy link
Member

Nooo37 commented Jul 7, 2021

Yea it doesn't seem to be a big performance issue (?) but it surely should be fixed. IDs are a cool idea but from what I've heard, they are designed specifically with tasklists and taglists in mind and thus don't really fit for user configs (might be wrong here) but they aren't really needed anyway because you can just keep the textbox or whatever as a var and wrap widgets around that var.

So I think there are two general solutions to this problem:

  • Either we draw the titlebar only once (so the create function gets only called once per client). And everything that needs to update in the titlebar should be handled by connecting it to signals. Then we could just keep the title-changed-signal but we would have to add signals for the tabbing group (Signal support for scratchpads #67) so that the titlebars can get notified when clients get added/removed from the tabbing group and update accordingly.
  • Or we don't connect to signals when drawing the titlebar. Then we would have to connect to the title-changed-signal (and possibly some more) right after the client gets added to the tabbing group and run the tabbed.update_tabbar function everytime the signal got emitted.

I would consider the second solution to be a quick fix and the first solution to be the better one but it would also require more effort creating signal support for tabgroups so I would be ok with both for now as I probably won't get around to fixing it myself within the next couple of weeks

@Nooo37 Nooo37 added the bug Something isn't working label Jul 7, 2021
@basaran
Copy link
Author

basaran commented Jul 8, 2021

Either would work, I wanted to create a tabbed module for layout-machi and went with the variable approach. I ended up creating a global table, and assigned the tabs to it which had the IDs. I then assigned the index of the table to the layout. Since the table keeps a pointer to the original tabs, it worked out pretty good so far. Here's some code:

...
  global_widget_table[cl.window][cc_ix] = tabs.create(cc, (cc == cl), buttons, cl_ix)
  flexlist:add(global_widget_table[cl.window][cc_ix])

Then when the tab group receives focus I could highlight the stuff, without cascading a series of signals.

local function focus_signal(c)
   if global_widget_table[c.window] then
      for i, p in pairs(global_widget_table[c.window]) do
         if p.focused then
            local widget = global_widget_table[c.window][i]:get_children_by_id(c.window)[1]
            widget.bg = "#43417a"
         end
      end
   end
end

----------------------------------------------------;

local function unfocus_signal(c)
   if global_widget_table[c.window] then
      for i, p in pairs(global_widget_table[c.window]) do
         if p.focused then
            p.bg = "#292929"
            break
         end
      end
   end
end

Same logic can be applied to the name signal to update the specific tab group as I'm keeping track of all the window IDs and I kind of know where those windows are. The whole thing is here (I update it daily, I refactor over the weekends):

https://github.com/basaran/awesomewm-machina

and this is how they look like (you can see the light grayish shade of the unfocused group, and the purple shade of the focus group)

image

P.S Your module gave me the idea of using titlebars as tabs. Thank you :) I didn't really want to mess with tasklists and position them into machi-layout, and this approach worked just fine and now there are tabs everywhere. Keeping the order of these tabs also required some creativity but I also took care of it with a global window list, and I would match the order from there.

I could keep it time sorted so new clients get added to the end of the queue, but I didn't want to complicate things too much at this time while I'm still adding features to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants