diff --git a/modules/Element.lua b/modules/Element.lua index 6099c18..24e4470 100644 --- a/modules/Element.lua +++ b/modules/Element.lua @@ -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 diff --git a/modules/LayoutEngine.lua b/modules/LayoutEngine.lua index 87a10bb..465e440 100644 --- a/modules/LayoutEngine.lua +++ b/modules/LayoutEngine.lua @@ -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 diff --git a/modules/ScrollManager.lua b/modules/ScrollManager.lua index b834f09..a783910 100644 --- a/modules/ScrollManager.lua +++ b/modules/ScrollManager.lua @@ -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 diff --git a/modules/types.lua b/modules/types.lua index 559e1bc..2fa4af1 100644 --- a/modules/types.lua +++ b/modules/types.lua @@ -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 diff --git a/testing/__tests__/scrollbar_placement_test.lua b/testing/__tests__/scrollbar_placement_test.lua new file mode 100644 index 0000000..12de530 --- /dev/null +++ b/testing/__tests__/scrollbar_placement_test.lua @@ -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 diff --git a/testing/runAll.lua b/testing/runAll.lua index 4bfd5c7..6809adc 100644 --- a/testing/runAll.lua +++ b/testing/runAll.lua @@ -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",