Implement overflow layout

[sclu1034]

This layout adds overflow behavior:

• adds a scrollbar
• adds scrollbar dragging to scroll
• adds mouse scroll behavior
• allows children to render at (near) infinite size in scroll direction

TODOs

• [x] Implement
• [x] Write docs
• [x] Write tests
• [x] Write examples
• [ ] SHOWSTOPPER: Wait for #3531 and adjust :layout to whatever comes from that
• [ ] Investigate compatibility of nested layouts
• [ ] Investigate per-widget scroll mode, i.e. "scroll step" becomes "scroll n-th next widget into view"
• [ ] Allow forcing the scrollbar to always be displayed
• [ ] Margin between content and scrollbar
• [ ] Styling options for the scrollbar background
  • Background color at the very least
  • Maybe also a generic widget slot to allow for stuff like hover effects

Demonstration

https://user-images.githubusercontent.com/4508454/113277319-e3f8e000-92e0-11eb-8610-f89ce8ada1f7.mp4

[codecov[bot]]

Codecov Report

Merging #3309 (d84dc1e) into master (70524e7) will increase coverage by 0.32%. The diff coverage is 95.26%.

@@            Coverage Diff             @@
##           master    #3309      +/-   ##
==========================================
+ Coverage   90.66%   90.99%   +0.32%     
==========================================
  Files         852      904      +52     
  Lines       54440    57502    +3062     
==========================================
+ Hits        49358    52323    +2965     
- Misses       5082     5179      +97     

Flag	Coverage Δ
gcov	90.99% <95.26%> (+0.32%)	:arrow_up:
luacov	93.70% <95.26%> (+0.20%)	:arrow_up:

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

Impacted Files	Coverage Δ
lib/wibox/layout/overflow.lua	82.83% <82.83%> (ø)
lib/wibox/hierarchy.lua	99.47% <100.00%> (+0.01%)	:arrow_up:
lib/wibox/layout/fixed.lua	71.71% <100.00%> (+0.94%)	:arrow_up:
lib/wibox/layout/init.lua	100.00% <100.00%> (ø)
spec/wibox/layout/overflow_spec.lua	100.00% <100.00%> (ø)
tests/examples/wibox/layout/defaults/overflow.lua	100.00% <100.00%> (ø)
...ests/examples/wibox/layout/overflow/fill_space.lua	100.00% <100.00%> (ø)
...mples/wibox/layout/overflow/scrollbar_position.lua	100.00% <100.00%> (ø)
...xamples/wibox/layout/overflow/scrollbar_widget.lua	100.00% <100.00%> (ø)
...examples/wibox/layout/overflow/scrollbar_width.lua	100.00% <100.00%> (ø)
... and 228 more

[HumblePresent]

I have done some testing with this layout and have encountered a couple issues/unexpected behavior.

1. A widget like the one below will not be shown in the overflow layout but is shown in a normal fixed layout

local test_layout = wibox.layout.overflow.vertical()
local test_constraint = wibox.widget {
    test_layout,
    strategy = "exact",
    height = 600,
    widget = wibox.container.constraint
}

local test_widget = wibox.widget {
    {
        {
            {
                text = "Testing overflow layout",
                widget = wibox.widget.textbox
            },
            {
                text = "Testing overflow layout",
                widget = wibox.widget.textbox
            },
            layout = wibox.layout.fixed.vertical
        },
        widget = wibox.container.background
    },
    strategy = "exact",
    width = 300,
    widget = wibox.container.contraint
}

test_layout:insert(1, test_widget)

Interestingly, this slighty modified widget will be displayed

local test_layout = wibox.layout.overflow.vertical()
local test_constraint = wibox.widget {
    test_layout,
    strategy = "exact",
    height = 600,
    widget = wibox.container.constraint
}

local test_widget = wibox.widget {
    {
        {
            text = "Testing overflow layout",
            widget = wibox.widget.textbox
        },
        widget = wibox.container.background
    },
    strategy = "exact",
    width = 300,
    widget = wibox.container.contraint
}

test_layout:insert(1, test_widget)

2. If the layout is constrained by a maximum size rather than exact, the scrollbar will sometimes be briefly visible when a widget is added to the layout as the layout expands with the added widgets until overflow. After overflow the scrollbar is always visible so the issue is mitigated. From my own debugging this appears to be caused by the way the fit() and layout() methods are called each time a widget is added. Here is a concrete example.

local test_layout = wibox.layout.overflow.vertical()
local test_constraint = wibox.widget {
    test_layout,
    strategy = "max",
    height = 600,
    widget = wibox.container.constraint
}

local test_widget = wibox.widget {
    {
        {
            text = "Testing overflow layout",
            widget = wibox.widget.textbox
        },
        widget = wibox.container.background
    },
    strategy = "exact",
    width = 300,
    height = 200,
    widget = wibox.container.contraint
}

-- Link to a key mapping or other event to insert multiple widgets
test_layout:insert(1, test_widget)

Here are some of the logs from inserting a widget into the overflow layout

Call to `overflow:fit()`
2021-12-16 23:09:14 W: awesome: orig_width: 9999, orig_height: 469
2021-12-16 23:09:14 W: awesome: used_max: 0, used_in_dir: 0

Call to `overflow:layout()`
2021-12-16 23:09:14 W: awesome: orig_width: 1, orig_height: 1
2021-12-16 23:09:14 W: awesome: used_in_dir: 156, avail_in_dir: 1
2021-12-16 23:09:14 W: awesome: Need scrollbar: true

Call to `overflow:layout()`
2021-12-16 23:09:14 W: awesome: orig_width: 234, orig_height: 156
2021-12-16 23:09:14 W: awesome: used_in_dir: 156, avail_in_dir: 156
2021-12-16 23:09:14 W: awesome: Need scrollbar: false

Due to the first call to overflow:layout() causing the scrollbar to be placed in the layout but then being removed right after by the second call there is an effect where the scrollbar "flashes" on the screen. The scope of this issue might be beyond the overflow layout and also have to do with the widget hierarchy but I included the info here anyway.

[sclu1034]

1. I'm not quite sure what's supposed to be going on here. Your layout is set to height = 600, the child widget to height = 300. 300 < 600, so there is no need for overflow behavior, and the layout will "fall back" to being a regular wibox.layout.fixed. So it sounds like this is working as intended.

2.

Call to `overflow:fit()`
2021-12-16 23:09:14 W: awesome: used_max: 0, used_in_dir: 0

Is this really logged after the child widget has been added? And is it logged at the end of :fit? overflow:fit shouldn't return 0, 0 when there is a child widget with a size > 0.

And this line looks weird as well

Call to `overflow:layout()`
2021-12-16 23:09:14 W: awesome: orig_width: 1, orig_height: 1

The overflow layout merely emits widget::layout_changed on :add/:insert. This looks like there's something weird going on in the widget system that calls :layout with (1, 1) and then again with the correct values. Possibly related to wibox.container.constrained and the different values for strategy, as you indicated.

[HumblePresent]

For 1 the height of the child widget is actually unbounded, the width is set to 300. The issue is not about overflow but that the child widget is not drawn in the layout at all. If I change the layout to a wibox.layout.fixed.vertical it is drawn so I would expect wibox.layout.overflow.vertical to also display it.

For 2 you are right I had the logs in the wrong spot. Here is the corrected version

Call to `overflow:fit()`
2021-12-17 09:41:38 W: awesome: num_widgets: 1
2021-12-17 09:41:38 W: awesome: orig_width: 9999, orig_height: 469
2021-12-17 09:41:38 W: awesome: used_max: 234, used_in_dir: 156

Call to `overflow:layout()`
2021-12-17 09:41:38 W: awesome: orig_width: 1, orig_height: 1
2021-12-17 09:41:38 W: awesome: used_in_dir: 156, avail_in_dir: 1
2021-12-17 09:41:38 W: awesome: Need scrollbar: true

Call to `overflow:layout()`
2021-12-17 09:41:38 W: awesome: orig_width: 234, orig_height: 156
2021-12-17 09:41:38 W: awesome: used_in_dir: 156, avail_in_dir: 156
2021-12-17 09:41:38 W: awesome: Need scrollbar: false

I agree the issue for 2 seems to be related to the widget system hierarchy rather than this layout implementation.

[sclu1034]

1. Thanks for the elaboration. The problem seems to be that wibox.layout.fixed:fit subtracts widget size from orig_height to get the total used, rather than adding from 0. And in this case, in orig_height - height_left, both values are math.huge (subtracting from Infinity is still Infinity), and the widget reports a height of 0.

[sclu1034]

Should now be fixed, see the added test case in overflow_spec.lua.

[HumblePresent]

So that does seem resolve issue 1 but I think the changes to hierarchy.lua and fixed.lua have broken some of my other widgets. Parts of my notification widgets that use wibox.layout.fixed no longer display.

[sclu1034]

The changes to hierarchy.lua don't affect rendering. That's about restricting click event target detection and due to the condition, the behaviour is unchanged for everything other than the overflow layout. For fixed.lua, I would need details. The test suite suggests no change in the layout's behaviour.

[HumblePresent]

Ok I think the issue presents when one or more of the child widgets in a wibox.layout.fixed layout has the visible property set the false. I noticed the issue in my notification widget so that is my test case. Here is the function I used to generate a notification widget template.

local function create_template(n)
    local template = {
        {
            {
                {
                    text = n.app_name,
                    widget = wibox.widget.textbox
                },
                {
                    text = n.title,
                    widget = wibox.widget.textbox
                },
                {
                    text = n.message,
                    widget = wibox.widget.textbox
                },
                layout = wibox.layout.fixed.vertical
            },
            bg = beautiful.bg_normal,
            widget = wibox.container.background
        },
        strategy = "max",
        height = 275,
        width = 375,
        widget = wibox.container.constraint
    }

    return template
end

If I then generate a notification it will be properly displayed according to the template above. If I set the visible property of one of the texboxes in the vertical fixed layout to false the notification will not be displayed.

local function create_template(n)
    local template = {
        {
            {
                {
                    text = n.app_name,
                    visible = false,
                    widget = wibox.widget.textbox
                },
                {
                    text = n.title,
                    widget = wibox.widget.textbox
                },
                {
                    text = n.message,
                    widget = wibox.widget.textbox
                },
                layout = wibox.layout.fixed.vertical
            },
            bg = beautiful.bg_normal,
            widget = wibox.container.background
        },
        strategy = "max",
        height = 275,
        width = 375,
        widget = wibox.container.constraint
    }

    return template
end

I also verified that this issue is only present after your latest commit 51424b5

[HumblePresent]

I've looked through the code a bit and I believe the issue is caused by the changes in the wibox.layout.fixed:fit() method. While iterating through the widgets in the layout, if the call to base.fit_widget() returns zero for the dimension in the direction of the layout, the iteration ends and fit() returns. The problem is that base.fit_widget() returns 0, 0 for any widget whose visible property is false, so any widgets in the layout after a widget that is not visible will be ignored.

[sclu1034]

The problem is that base.fit_widget() returns 0, 0 for any widget whose visible property is false

Oh, that's bad. I didn't realize until now, but base.fit_widget actually encodes multiple different things into 0, 0.

When spacing is involved, it actually makes a difference whether a widget is visible = false and both the widget and the spacing should be skipped, or if the widget only reports 0, 0 due to some internal state but should still be shown (or rather, should still be accounted for with spacing). Similarly, if the layout doesn't have enough space to fit all children, currently the only way a child widget can report "that's not enough space for me" is, again, returning 0, 0. But that's the condition where wibox.layout.fixed should break the loop.

[HumblePresent]

currently the only way a child widget can report "that's not enough space for me" is, again, returning 0, 0

Can't child widgets return a size beyond what's available in the layout to indicate there isn't enough space? I thought fit() was supposed to return a preferred size even if that's not what they end up receiving.

[Elv13]

I don't know if it's relevant, but that reminds me of that bug https://github.com/awesomeWM/awesome/commit/83c31f948b57f3dfb75d343a084ef1509d740902

The 0x0 widgets are tricky to handle...

[sclu1034]

Can't child widgets return a size beyond what's available in the layout to indicate there isn't enough space? I thought fit() was supposed to return a preferred size even if that's not what they end up receiving.

Sure, that might be slightly less ambiguous. But that's still using a return value of two numbers that could be valid to encode information that shouldn't be encoded with numbers. And it wouldn't help at all with the visible = false part.

[sclu1034]

I've extracted the discussion about base.fit_widget into a separate issue, since this affects all layouts.

[HumblePresent]

I have another instance of unexpected behavior. I know I'm a bit of a thorn in the side of this PR right now, but better to work these things out now rather than later when someone creates an issue right :grin:.

When I click on the scrollbar without moving the mouse the scrollbar moves and the scroll position changes. As soon as I move the mouse a little the scrollbar and and scroll position seem to snap back to their original location before being clicked on. I would expect the scrollbar to behave in a typical manner where it does not move until the mouse is moved.

[HumblePresent]

Just tested again and that last change fixed my issue.

[sclu1034]

do you think it's now ready?

That would be up to the reviewers to decide. It's still affected by #3531, so it's "only" as much broken as all the other layouts.

it will be marked as "inherited from widget.base" and the user will ignore it, like they already ignore "children" on most widgets.

That's not a good comparison. .children is a user-facing API (changing child widgets is the whole point of a layout, after all). However, .clip_child_extends is not a user-facing API, just like :fit or :layout. A consumer of a widget should not change that property on the widget, only the widget's' implementation. So, just like how :fit doesn't show up in the "Object methods" section, clip_child_extends should not show up in the "Object properties" section.

Would prefixing it with _ help with marking it as private?

[Elv13]

Sorry for the delay, I didn't work on AwesomeWM since xmas. I did some testing and it works. My earlier comments are addressed and the API looks good. Thanks a lot for this.

Now, yes, this container has some unfixeable limitations, but it provides a lot of value for people who don't use the more advanced widget system features. Thanks a lot for this contribution!

[HumblePresent]

Before this gets merged I do want to point out that the changes to fixed.lua do still break fixed layouts with any child widgets that have visible = false. By break I mean widgets that come after an "invisible" widget in the layout will not be shown regardless of their visible property. Is it possible to merge this PR without the changes to fixed.lua and make any necessary changes after #3531 has been resolved?

[sclu1034]

As far as I can see, we have three options:

• Get #3531 fixed, which the current issue here blocks on
• Revert changes and reintroduce the bug described as 1. above (wibox.layout.overflow will be somewhat broken after merge)
• Keep the current bug with stopping on the first visible = false (regression in wibox.layout.fixed)

Ideally, we would do the first. However, if you do want this PR merged now, the second option would be preferable.

[sammy-ette]

after using this myself i have noticed 2 problems:

• the doc entry is wrong for the vertical layout (https://safe.kashima.moe/qi0g6vvj0jq7.png)
• :reset() does not reset the scroll position

havent used it enough to notice any other problems, besides maybe the default step value being too low in my opinion

[Crylia]

I noticed when using two overflow containers (one inside another one) that both get scrolled, when scrolling with the mouse wheel. When scrolling you should probably check which is the nearest overflow widget under the cursor and only scroll that one.

[sammy-ette]

some other things i recommend adding:

• background for the scrollbar widget
• spacing between contents and scrollbar (an easier way to do that)

[sclu1034]

For now this is fully blocked on #3531 .