feat: optionally position scrollbar over menu

[willothy]

I prefer a floating scrollbar like I had originally implemented - this adds a "floating" option to use no background window, and an option to disable the scrollbar entirely. The current behavior is still the default.

[willothy]

Rebased on top of master and updated docs :)

[Bekaboo]

Hi, thanks for this PR. We could achieve further customizability by allowing user to control the window configurations of the scrollbar and enable/disable the scrollbar thumb and the background separately, like this:

local opts = {
  menu = {
    scrollbar = {
      enable = true,
        sbar = {
          enable = true,
          ---@alias dropbar_menu_win_config_opts_t any|fun(menu: dropbar_menu_t):any
          ---@type table<string, dropbar_menu_win_config_opts_t>
          ---@see vim.api.nvim_open_win
          win_conifgs = {
          }
        },
        thumb = {
          enable = true,
          ---@type table<string, dropbar_menu_win_config_opts_t>
          win_conifgs = {
          }
        }
      }
    }
  }
}

[willothy]

That's a good idea, I'll make those changes tomorrow!

[willothy]

My bad, some things came up and I didn't have time to get to this, but I should have some time in the next few days :)

[Bekaboo]

@willothy Sure, please take your time, we are not in a hurry! Thanks for the great PRs you've made along the way. I can give you push permission if that is more convenient for you.

[willothy]

@Bekaboo sure, that'd be cool! Thanks for trusting me with that :) I'll still wait for reviews before merging anything of course.

[Bekaboo]

I have sent you an invitation.

[willothy]

Thanks, sounds good!

[willothy]

Working on this right now - I think it would be cool to be able to disable the bar and thumb separately, but I'm not sure I see a point in allowing users to customize win_configs. The position of the scrollbar should always be to the right of the menu, so it would likely end up ignoring all options except border, which I doubt people will use for a scrollbar. What do you think?

I think something like this is probably enough, and more properties could be added individually (maybe blend or winhl) if needed.

scrollbar = {
  enabled = true,
  -- The background / gutter of the scrollbar (optional)
  sbar = {
    -- When false, only the thumb is shown.
    enabled = true,
  },
},

[Bekaboo]

The position of the scrollbar should always be to the right of the menu, so it would likely end up ignoring all options except border

This makes sense, to reduce the complexity we can only make the border configurable. However we can still have the win_configs field but the only valid sub-field is border, in case we want to make other sub-fields configurable in the future. This must be clarified in the README and vimdoc to avoid confusion.

Also, the enable field should have type fun(menu: dropbar_menu_t)|boolean. The default value of sbar.enable can be a function that returns true if the border of menu is none else false, to avoid covering the right border of the menu, like how nvim-cmp's scrollbar works.

---

There is an existing bug -- when a dropbar menu has border, the scrollbar and scroll menu will be covered by the border.

To fix the issue we have to set a different z index for the scrollbar such that it is above the menu but it should be covered by the sub-menus, e.g. if the first menu has zindex = 50, the scrollbar has zindex = 51, then the first sub-menu should also have zindx = 51 with its scrollbar having zindex = 51, etc. We can fix this in a separate commit.

[Bekaboo]

Also, the enable field should have type fun(menu: dropbar_menu_t)|boolean. The default value of sbar.enable can be a function that returns true if the border of menu is none else false, to avoid covering the right border of the menu, like how nvim-cmp's scrollbar works.

I think the reason why nvim-cmp's scrollbar has no background is simply that the background has the same zindex as the completion menu, so it is covered by the border of completion menu (if any), but the thumb has a higher zindex and is thus not covered.

---

EDIT:

zindex issue should be fixed in 2b7c2d5, the scrollbar thumb will have a higher zindex than the menu while the sbar (background) will have the same zindex so that it will be covered by the menu border, if any.

Also, the enable field should have type fun(menu: dropbar_menu_t)|boolean. The default value of sbar.enable can be a function that returns true if the border of menu is none else false, to avoid covering the right border of the menu, like how nvim-cmp's scrollbar works.

To avoid complexity the enable field should have type boolean, we will not allow a callback. @willothy What is your opinion?

[willothy]

zindex issue should be fixed in 5ace799, the scrollbar thumb will have a higher zindex than the menu while the sbar (background) will have the same zindex so that it will be converted by the menu border, if any.

Nice, that looks really cool! I may have to start using a border in my config...

Also, the enable field should have type fun(menu: dropbar_menu_t)|boolean. The default value of sbar.enable can be a function that returns true if the border of menu is none else false, to avoid covering the right border of the menu, like how nvim-cmp's scrollbar works.

To avoid complexity the enable field should have type boolean, we will not allow a callback. @willothy What is your opinion?

I agree with the enable field just being a boolean, I think the border logic you just added is all that's needed since that's probably what people would use a callback for. If more customization is needed, a callback would be easy to add in the future anyways.

This makes sense, to reduce the complexity we can only make the border configurable. However we can still have the win_configs field but the only valid sub-field is border, in case we want to make other sub-fields configurable in the future. This must be clarified in the README and vimdoc to avoid confusion.

That sounds good. Should the border be allowed on the background as well, or just on the thumb? There may be some edge cases to handle to ensure that scrollbar borders don't overlap with submenus.

[Bekaboo]

That sounds good. Should the border be allowed on the background as well, or just on the thumb? There may be some edge cases to handle to ensure that scrollbar borders don't overlap with submenus.

After some reflection, I realized that no one has really requested to customize the scrollbar border, and only allowing border to be configurable in the win_configs table is wired. If you need to customize it you can add it, if not I suggest drop this, so the config spec will look like the following:

scrollbar = {
  enabled = true,
  -- The background / gutter of the scrollbar (optional)
  sbar = {
    enabled = true, -- should we remove this too? it is still needed if you want to remove the background when menu border is 'none'
  },
},

[willothy]

After some reflection, I realized that no one has really requested to customize the scrollbar border, and only allowing border to be configurable in the win_configs table is wired.

Yeah, that's good with me, makes things a lot simpler. I wouldn't be using the border, so no worries. Not sure why I had that idea lol.

sbar = {
  enabled = true, -- should we remove this too? it is still needed if you want to remove the background when menu border is 'none'
},

I do want to still be able to remove the background when there's no border though, I think that's an important customization (or one that I'll use, at least haha). Maybe it can just be a single boolean, like sbar = true or background = true instead of a table?

[Bekaboo]

Maybe it can just be a single boolean, like sbar = true or background = true instead of a table?

Agreed. I suggest the following structure:

menu = {
  scrollbar = {
    enable = true,
    background = true
  }
}

[willothy]

Maybe it can just be a single boolean, like sbar = true or background = true instead of a table?

Agreed. I suggest the following structure:

menu = {
  scrollbar = {
    enable = true,
    background = true
  }
}

Sounds good! I'll update and fix the conflicts tomorrow.

[willothy]

Should be all good now

[Bekaboo]

@willothy Thanks for the PR, looks great!

A few small suggestions:

1. Use enable not enabled, for consistency with general.enable
2. New options should be documented under README Menu section and also in the vimdoc
3. Any reason to get the width of the menu window using nvim_win_get_width() instead of getting it from menu_win_configs.width on line 573 in menu.lua?
4. After 2b7c2d5, the background (sbar) is created first, then the thumb. Could you also reorder it in the PR? Sorry for the inconvenience.

[willothy]

1. Use enable not enabled, for consistency with general.enable

Done

2. New options should be documented under README Menu section and also in the vimdoc

Added vimdoc and updated the scrollbar docs in readme with the new names.

3. Any reason to get the width of the menu window using nvim_win_get_width() instead of getting it from menu_win_configs.width on line 573 in menu.lua?

Not sure, but I removed it because it's not needed now lol

4. After 2b7c2d5, the background (sbar) is created first, then the thumb. Could you also reorder it in the PR? Sorry for the inconvenience.

No worries! Should be done as well.