fix scrollbar spacing issue

This commit is contained in:
Michael Freno
2026-01-06 00:12:21 -05:00
parent 49f37a1bb0
commit ce690aa5dc
6 changed files with 270 additions and 4 deletions

View File

@@ -140,6 +140,7 @@
---@field invertScroll boolean? -- Invert mouse wheel scroll direction (default: false)
---@field scrollBarStyle string? -- Scrollbar style name from theme (selects from theme.scrollbars)
---@field scrollbarKnobOffset number|table? -- Scrollbar knob/handle offset (number or {x, y} or {horizontal, vertical})
---@field scrollbarPlacement string? -- "reserve-space"|"overlay" -- Whether scrollbar reserves space or overlays content (default: "reserve-space")
---@field _overflowX boolean? -- Internal: whether content overflows horizontally
---@field _overflowY boolean? -- Internal: whether content overflows vertically
---@field _contentWidth number? -- Internal: total content width
@@ -1939,6 +1940,7 @@ function Element.new(props)
scrollBarStyle = props.scrollBarStyle,
scrollbarKnobOffset = props.scrollbarKnobOffset,
hideScrollbars = props.hideScrollbars,
scrollbarPlacement = props.scrollbarPlacement,
_scrollX = props._scrollX,
_scrollY = props._scrollY,
}, scrollManagerDeps)
@@ -1957,6 +1959,7 @@ function Element.new(props)
self.scrollBarStyle = self._scrollManager.scrollBarStyle
self.scrollbarKnobOffset = self._scrollManager.scrollbarKnobOffset
self.hideScrollbars = self._scrollManager.hideScrollbars
self.scrollbarPlacement = self._scrollManager.scrollbarPlacement
-- Initialize state properties (will be synced from ScrollManager)
self._overflowX = false

View File

@@ -445,12 +445,48 @@ function LayoutEngine:layoutChildren()
-- BORDER-BOX MODEL: element.width and element.height are already content dimensions (padding subtracted)
local availableMainSize = 0
local availableCrossSize = 0
-- Reserve space for scrollbars if needed (reserve-space mode)
local scrollbarReservedWidth = 0
local scrollbarReservedHeight = 0
if self.element._scrollManager and self.element._scrollManager.scrollbarPlacement == "reserve-space" then
scrollbarReservedWidth, scrollbarReservedHeight = self.element._scrollManager:getReservedSpace(self.element)
end
if self.flexDirection == self._FlexDirection.HORIZONTAL then
availableMainSize = self.element.width
availableCrossSize = self.element.height
availableMainSize = self.element.width - scrollbarReservedWidth
availableCrossSize = self.element.height - scrollbarReservedHeight
else
availableMainSize = self.element.height
availableCrossSize = self.element.width
availableMainSize = self.element.height - scrollbarReservedHeight
availableCrossSize = self.element.width - scrollbarReservedWidth
end
-- Adjust children with percentage-based cross-axis dimensions when scrollbar space is reserved
if (scrollbarReservedWidth > 0 or scrollbarReservedHeight > 0) then
local isHorizontal = self.flexDirection == self._FlexDirection.HORIZONTAL
for _, child in ipairs(flexChildren) do
if isHorizontal then
-- Horizontal flex: cross-axis is height
if child.units and child.units.height and child.units.height.unit == "%" then
-- Re-resolve percentage height against reduced cross-axis size
-- The percentage applies to border-box, so we need to subtract padding to get content height
local newBorderBoxHeight = (child.units.height.value / 100) * availableCrossSize
local newHeight = math.max(0, newBorderBoxHeight - child.padding.top - child.padding.bottom)
child.height = newHeight
child._borderBoxHeight = newBorderBoxHeight
end
else
-- Vertical flex: cross-axis is width
if child.units and child.units.width and child.units.width.unit == "%" then
-- Re-resolve percentage width against reduced cross-axis size
-- The percentage applies to border-box, so we need to subtract padding to get content width
local newBorderBoxWidth = (child.units.width.value / 100) * availableCrossSize
local newWidth = math.max(0, newBorderBoxWidth - child.padding.left - child.padding.right)
child.width = newWidth
child._borderBoxWidth = newBorderBoxWidth
end
end
end
end
-- Handle flex wrap: create lines of children

View File

@@ -12,6 +12,7 @@
---@field scrollBarStyle string? -- Scrollbar style name from theme (selects from theme.scrollbars)
---@field scrollbarKnobOffset table -- {x: number, y: number, horizontal: number, vertical: number} -- Offset for scrollbar knob/handle position
---@field hideScrollbars table -- {vertical: boolean, horizontal: boolean}
---@field scrollbarPlacement string -- "reserve-space"|"overlay" -- Whether scrollbar reserves space or overlays content (default: "reserve-space")
---@field touchScrollEnabled boolean -- Enable touch scrolling
---@field momentumScrollEnabled boolean -- Enable momentum scrolling
---@field bounceEnabled boolean -- Enable bounce effects at boundaries
@@ -98,6 +99,9 @@ function ScrollManager.new(config, deps)
-- hideScrollbars can be boolean or table {vertical: boolean, horizontal: boolean}
self.hideScrollbars = self._utils.normalizeBooleanTable(config.hideScrollbars, false)
-- Scrollbar placement: "reserve-space" (default) or "overlay"
self.scrollbarPlacement = config.scrollbarPlacement or "reserve-space"
-- Touch scrolling configuration
self.touchScrollEnabled = config.touchScrollEnabled ~= false -- Default true
self.momentumScrollEnabled = config.momentumScrollEnabled ~= false -- Default true
@@ -146,6 +150,34 @@ function ScrollManager.new(config, deps)
return self
end
--- Get the space reserved for scrollbars (width and height reduction)
--- This is called BEFORE layout to reduce available space for children
---@param element Element The parent Element instance
---@return number reservedWidth, number reservedHeight
function ScrollManager:getReservedSpace(element)
if self.scrollbarPlacement ~= "reserve-space" then
return 0, 0
end
local overflowX = self.overflowX or self.overflow
local overflowY = self.overflowY or self.overflow
local reservedWidth = 0
local reservedHeight = 0
-- Reserve space for vertical scrollbar if overflow mode requires it
if (overflowY == "scroll" or overflowY == "auto") and not self.hideScrollbars.vertical then
reservedWidth = self.scrollbarWidth + (self.scrollbarPadding * 2)
end
-- Reserve space for horizontal scrollbar if overflow mode requires it
if (overflowX == "scroll" or overflowX == "auto") and not self.hideScrollbars.horizontal then
reservedHeight = self.scrollbarWidth + (self.scrollbarPadding * 2)
end
return reservedWidth, reservedHeight
end
--- Detect if content overflows container bounds
---@param element Element The parent Element instance
function ScrollManager:detectOverflow(element)
@@ -199,6 +231,14 @@ function ScrollManager:detectOverflow(element)
local containerWidth = element.width - element.padding.left - element.padding.right
local containerHeight = element.height - element.padding.top - element.padding.bottom
-- If scrollbarPlacement is "reserve-space", we need to subtract the reserved space
-- because the layout already accounted for it, but element.width/height are still full size
if self.scrollbarPlacement == "reserve-space" then
local reservedWidth, reservedHeight = self:getReservedSpace()
containerWidth = containerWidth - reservedWidth
containerHeight = containerHeight - reservedHeight
end
self._overflowX = self._contentWidth > containerWidth
self._overflowY = self._contentHeight > containerHeight
@@ -674,6 +714,7 @@ function ScrollManager:getState()
_scrollbarHoveredHorizontal = self._scrollbarHoveredHorizontal or false,
scrollBarStyle = self.scrollBarStyle,
scrollbarKnobOffset = self.scrollbarKnobOffset,
scrollbarPlacement = self.scrollbarPlacement,
_overflowX = self._overflowX,
_overflowY = self._overflowY,
_contentWidth = self._contentWidth,
@@ -752,6 +793,10 @@ function ScrollManager:setState(state)
self.scrollbarKnobOffset = self._utils.normalizeOffsetTable(state.scrollbarKnobOffset, 0)
end
if state.scrollbarPlacement ~= nil then
self.scrollbarPlacement = state.scrollbarPlacement
end
if state._overflowX ~= nil then
self._overflowX = state._overflowX
end

View File

@@ -67,6 +67,10 @@ local AnimationProps = {}
---@field alignItems AlignItems? -- Alignment of items along cross axis (default: STRETCH)
---@field alignContent AlignContent? -- Alignment of lines in multi-line flex containers (default: STRETCH)
---@field flexWrap FlexWrap? -- Whether children wrap to multiple lines: "nowrap"|"wrap"|"wrap-reverse" (default: NOWRAP)
---@field flex number|string? -- Shorthand for flexGrow, flexShrink, flexBasis: number (flex-grow only), string ("1 0 auto"), or nil (default: nil)
---@field flexGrow number? -- How much the element should grow relative to siblings (default: 0)
---@field flexShrink number? -- How much the element should shrink relative to siblings (default: 1)
---@field flexBasis number|string|CalcObject? -- Initial size before growing/shrinking: number (px), string ("50%", "10vw", "auto"), or CalcObject (default: "auto")
---@field justifySelf JustifySelf? -- Alignment of the item itself along main axis (default: AUTO)
---@field alignSelf AlignSelf? -- Alignment of the item itself along cross axis (default: AUTO)
---@field onEvent fun(element:Element, event:InputEvent)? -- Callback function for interaction events
@@ -126,6 +130,7 @@ local AnimationProps = {}
---@field smoothScrollEnabled boolean? -- Enable smooth scrolling animation for wheel events (default: false)
---@field scrollBarStyle string? -- Scrollbar style name from theme (selects from theme.scrollbars, default: uses first scrollbar or fallback rendering)
---@field scrollbarKnobOffset number|{x:number, y:number}|{horizontal:number, vertical:number}? -- Offset for scrollbar knob/handle position in pixels (number for both axes, or table for per-axis control, default: 0, adds to theme offset)
---@field scrollbarPlacement "reserve-space"|"overlay"? -- Scrollbar rendering mode: "reserve-space" (reduces content area, default) or "overlay" (renders over content)
---@field hideScrollbars boolean|{vertical:boolean, horizontal:boolean}? -- Hide scrollbars (boolean for both, or table for individual control, default: false)
---@field imagePath string? -- Path to image file (auto-loads via ImageCache)
---@field image love.Image? -- Image object to display

View File

@@ -0,0 +1,176 @@
package.path = package.path .. ";./?.lua;./modules/?.lua"
local originalSearchers = package.searchers or package.loaders
table.insert(originalSearchers, 2, function(modname)
if modname:match("^FlexLove%.modules%.") then
local moduleName = modname:gsub("^FlexLove%.modules%.", "")
return function()
return require("modules." .. moduleName)
end
end
end)
require("testing.loveStub")
local luaunit = require("testing.luaunit")
local FlexLove = require("FlexLove")
FlexLove.init()
TestScrollbarPlacement = {}
function TestScrollbarPlacement:setUp()
FlexLove.setMode("retained")
end
function TestScrollbarPlacement:test_reserve_space_with_percentage_height_children()
-- Test case from user: horizontal scroll container with 100% height children
-- Should NOT cause vertical overflow
local container = FlexLove.new({
width = 200,
height = 200,
positioning = "flex",
flexDirection = "horizontal",
overflow = "scroll", -- Always shows scrollbars
scrollbarPlacement = "reserve-space",
})
local child1 = FlexLove.new({
width = 100,
height = "100%",
parent = container,
})
-- Trigger layout
container:layoutChildren()
container._scrollManager:detectOverflow(container)
local overflowX, overflowY = container._scrollManager:hasOverflow()
-- Child height should be reduced to account for horizontal scrollbar
-- Default scrollbar is 12px + 2px padding on each side = 16px
-- So child height should be 200 - 16 = 184px
luaunit.assertEquals(child1.height, 184)
-- Should have horizontal overflow (scroll mode), but NOT vertical
luaunit.assertFalse(overflowY, "Should not have vertical overflow with 100% height child")
end
function TestScrollbarPlacement:test_reserve_space_with_percentage_width_children()
-- Vertical scroll container with 100% width children
local container = FlexLove.new({
width = 200,
height = 200,
positioning = "flex",
flexDirection = "column",
overflow = "scroll", -- Always shows scrollbars
scrollbarPlacement = "reserve-space",
})
local child1 = FlexLove.new({
width = "100%",
height = 100,
parent = container,
})
-- Trigger layout
container:layoutChildren()
container._scrollManager:detectOverflow(container)
local overflowX, overflowY = container._scrollManager:hasOverflow()
-- Child width should be reduced to account for vertical scrollbar
-- Default scrollbar is 12px + 2px padding on each side = 16px
-- So child width should be 200 - 16 = 184px
luaunit.assertEquals(child1.width, 184)
-- Should have vertical overflow (scroll mode), but NOT horizontal
luaunit.assertFalse(overflowX, "Should not have horizontal overflow with 100% width child")
end
function TestScrollbarPlacement:test_overlay_mode_no_size_adjustment()
-- Overlay mode should NOT adjust child sizes
local container = FlexLove.new({
width = 200,
height = 200,
positioning = "flex",
flexDirection = "horizontal",
overflow = "scroll",
scrollbarPlacement = "overlay",
})
local child1 = FlexLove.new({
width = 100,
height = "100%",
parent = container,
})
-- Trigger layout
container:layoutChildren()
-- Child height should be full 200px (no reduction for scrollbar)
luaunit.assertEquals(child1.height, 200)
end
function TestScrollbarPlacement:test_auto_overflow_reserves_space_only_when_needed()
-- With overflow="auto" and reserve-space mode, space is reserved preemptively
-- But scrollbars should only be visible when content actually overflows
local container = FlexLove.new({
width = 200,
height = 200,
positioning = "flex",
flexDirection = "column",
overflow = "auto",
scrollbarPlacement = "reserve-space",
})
-- Child that doesn't cause overflow
local child1 = FlexLove.new({
width = "100%",
height = 100,
parent = container,
})
-- Trigger layout and overflow detection
container:layoutChildren()
container._scrollManager:detectOverflow(container)
local overflowX, overflowY = container._scrollManager:hasOverflow()
-- No overflow detected since content (100px) < available height (184px after scrollbar reservation)
luaunit.assertFalse(overflowY, "Should not have overflow")
-- Space is reserved preemptively with overflow="auto" to avoid layout shifts
luaunit.assertEquals(child1.width, 184, "Space should be reserved preemptively with auto mode")
end
function TestScrollbarPlacement:test_vertical_overflow_detected_with_reserved_space()
-- Test that overflow is properly detected when using reserve-space
local container = FlexLove.new({
width = 200,
height = 200,
positioning = "flex",
flexDirection = "column",
overflow = "auto",
scrollbarPlacement = "reserve-space",
})
-- Child that WILL cause overflow
local child1 = FlexLove.new({
width = "100%",
height = 300,
parent = container,
})
-- Trigger layout and overflow detection
container:layoutChildren()
container._scrollManager:detectOverflow(container)
local overflowX, overflowY = container._scrollManager:hasOverflow()
-- Should detect vertical overflow
luaunit.assertTrue(overflowY, "Should detect vertical overflow")
-- Child width should be reduced for vertical scrollbar
luaunit.assertEquals(child1.width, 184, "Child width should be reduced for scrollbar")
end
if not _G.RUNNING_ALL_TESTS then
os.exit(luaunit.LuaUnit.run())
end

View File

@@ -63,6 +63,7 @@ local testFiles = {
"testing/__tests__/retained_prop_stability_test.lua",
"testing/__tests__/roundedrect_test.lua",
"testing/__tests__/scroll_manager_test.lua",
"testing/__tests__/scrollbar_placement_test.lua",
"testing/__tests__/text_editor_test.lua",
"testing/__tests__/theme_test.lua",
"testing/__tests__/touch_events_test.lua",