transition Label to use the new generic Scrollbar

docs/Lua API.rst

@@ -4084,11 +4084,6 @@ It has the following attributes:
     keys to the number of lines to scroll as positive or negative integers or one of the keywords
     supported by the ``scroll`` method. The default is up/down arrows scrolling by one line and page
     up/down scrolling by one page.
-:show_scrollbar: Controls scrollbar display: ``false`` for no scrollbar, ``'right'`` or ``'left'`` for
-    icons next to the text in an additional column (``frame_inset`` is adjusted to have ``.r`` or ``.l`` greater than ``0``),
-    ``nil`` same as ``'right'`` but changes ``frame_inset`` only if a scroll icon is actually necessary
-    (if ``getTextHeight()`` is greater than ``frame_body.height``). Default is ``nil``.
-:scrollbar: The table of attributes to pass to the `Scrollbar class`_.
 
 The text itself is represented as a complex structure, and passed
 to the object via the ``text`` argument of the constructor, or via
@@ -4181,7 +4176,8 @@ The Label widget implements the following methods:
 
   This method takes the number of lines to scroll as positive or negative
   integers or one of the following keywords: ``+page``, ``-page``,
-  ``+halfpage``, or ``-halfpage``.
+  ``+halfpage``, or ``-halfpage``. It returns the number of lines that were
+  actually scrolled (negative for scrolling up).
 
 WrappedLabel class
 ------------------

library/lua/gui/widgets.lua

@@ -636,12 +636,15 @@ Label.ATTRS{
     on_click = DEFAULT_NIL,
     on_rclick = DEFAULT_NIL,
     scroll_keys = STANDARDSCROLL,
-    show_scrollbar = DEFAULT_NIL, -- DEFAULT_NIL, 'right', 'left', false
-    scrollbar_fg = COLOR_LIGHTGREEN,
-    scrollbar_bg = COLOR_CYAN
 }
 
 function Label:init(args)
+    self.scrollbar = Scrollbar{
+        frame={r=0},
+        on_scroll=self:callback('on_scrollbar')}
+
+    self:addviews{self.scrollbar}
+
     -- use existing saved text if no explicit text was specified. this avoids
     -- overwriting pre-formatted text that subclasses may have already set
     self:setText(args.text or self.text)
@@ -650,6 +653,12 @@ function Label:init(args)
     end
 end
 
+local function update_label_scrollbar(label)
+    local body_height = label.frame_body and label.frame_body.height or 1
+    label.scrollbar:update(label.start_line_num, body_height,
+                           label:getTextHeight())
+end
+
 function Label:setText(text)
     self.start_line_num = 1
     self.text = text
@@ -659,81 +668,8 @@ function Label:setText(text)
         self.frame = self.frame or {}
         self.frame.h = self:getTextHeight()
     end
-end
 
-function Label:update_scroll_inset()
+    update_label_scrollbar(self)
-    if self.show_scrollbar == nil then
-        self._show_scrollbar = self:getTextHeight() > self.frame_body.height and 'right' or false
-    else
-        self._show_scrollbar = self.show_scrollbar
-    end
-    if self._show_scrollbar then
-        -- here self._show_scrollbar can only be either
-        -- 'left' or any true value which we interpret as right
-        local l,t,r,b = gui.parse_inset(self.frame_inset)
-        if self._show_scrollbar == 'left' and l <= 0 then
-            l = 1
-        elseif r <= 0 then
-            r = 1
-        end
-        self.frame_inset = {l=l,t=t,r=r,b=b}
-    end
-end
-
--- the position is the number of tiles of empty space above the top of the
--- scrollbar, and the height is the number of tiles the scrollbar should occupy
--- to represent the percentage of text that is on the screen.
-local function get_scrollbar_pos_and_height(label)
-    local first_visible_line = label.start_line_num
-    local text_height = label:getTextHeight()
-    local last_visible_line = first_visible_line + label.frame_body.height - 1
-    local scrollbar_body_height = label.frame_body.height - 2
-    local displayed_lines = last_visible_line - first_visible_line
-
-    local height = math.floor(((displayed_lines-1) * scrollbar_body_height) /
-                              text_height)
-
-    local max_pos = scrollbar_body_height - height
-    local pos = math.ceil(((first_visible_line-1) * max_pos) /
-                          (text_height - label.frame_body.height))
-
-    return pos, height
-end
-
-function Label:render_scrollbar(dc, x, y1, y2)
-    -- render up arrow if we're not at the top
-    dc:seek(x, y1):char(
-        self.start_line_num == 1 and NO_ARROW_CHAR or UP_ARROW_CHAR,
-        self.scrollbar_fg, self.scrollbar_bg)
-    -- render scrollbar body
-    local pos, height = get_scrollbar_pos_and_height(self)
-    local starty = y1 + pos + 1
-    local endy = y1 + pos + height
-    for y=y1+1,y2-1 do
-        if y >= starty and y <= endy then
-            dc:seek(x, y):char(BAR_CHAR, self.scrollbar_fg)
-        else
-            dc:seek(x, y):char(BAR_BG_CHAR, self.scrollbar_bg)
-        end
-    end
-    -- render down arrow if we're not at the bottom
-    local last_visible_line = self.start_line_num + self.frame_body.height - 1
-    dc:seek(x, y2):char(
-        last_visible_line >= self:getTextHeight() and
-            NO_ARROW_CHAR or DOWN_ARROW_CHAR,
-        self.scrollbar_fg, self.scrollbar_bg)
-end
-
-function Label:computeFrame(parent_rect)
-    local frame_rect,body_rect = Label.super.computeFrame(self, parent_rect)
-
-    self.frame_rect = frame_rect
-    self.frame_body = parent_rect:viewport(body_rect or frame_rect)
-
-    self:update_scroll_inset() -- frame_body is now set
-
-    -- recalc with updated frame_inset
-    return Label.super.computeFrame(self, parent_rect)
 end
 
 function Label:preUpdateLayout()
@@ -743,6 +679,10 @@ function Label:preUpdateLayout()
     end
 end
 
+function Label:postUpdateLayout()
+    update_label_scrollbar(self)
+end
+
 function Label:itemById(id)
     if self.text_ids then
         return self.text_ids[id]
@@ -766,44 +706,19 @@ function Label:onRenderBody(dc)
     render_text(self,dc,0,0,text_pen,self.text_dpen,is_disabled(self))
 end
 
-function Label:onRenderFrame(dc, rect)
+function Label:on_scrollbar(scroll_spec)
-    if self._show_scrollbar then
+    local v = 0
-        local x = self._show_scrollbar == 'left'
+    if scroll_spec == 'down_large' then
-                and self.frame_body.x1-dc.x1-1
+        v = '+halfpage'
-                or  self.frame_body.x2-dc.x1+1
+    elseif scroll_spec == 'up_large' then
-        self:render_scrollbar(dc,
+        v = '-halfpage'
-            x,
+    elseif scroll_spec == 'down_small' then
-            self.frame_body.y1-dc.y1,
+        v = 1
-            self.frame_body.y2-dc.y1
+    elseif scroll_spec == 'up_small' then
-        )
+        v = -1
-    end
-end
-
-function Label:click_scrollbar()
-    if not self._show_scrollbar then return end
-    local rect = self.frame_body
-    local x, y = dscreen.getMousePos()
-
-    if self._show_scrollbar == 'left' and x ~= rect.x1-1 or x ~= rect.x2+1 then
-        return
-    end
-    if y < rect.y1 or y > rect.y2 then
-        return
     end
 
-    if y == rect.y1 then
+    self:scroll(v)
-        return -1
-    elseif y == rect.y2 then
-        return 1
-    else
-        local pos, height = get_scrollbar_pos_and_height(self)
-        if y <= rect.y1 + pos then
-            return '-halfpage'
-        elseif y > rect.y1 + pos + height then
-            return '+halfpage'
-        end
-    end
-    return nil
 end
 
 function Label:scroll(nlines)
@@ -824,24 +739,28 @@ function Label:scroll(nlines)
     local n = self.start_line_num + nlines
     n = math.min(n, self:getTextHeight() - self.frame_body.height + 1)
     n = math.max(n, 1)
+    nlines = n - self.start_line_num
     self.start_line_num = n
+    update_label_scrollbar(self)
     return nlines
 end
 
 function Label:onInput(keys)
     if is_disabled(self) then return false end
-    if keys._MOUSE_L_DOWN then
+    if self:inputToSubviews(keys) then
-        if not self:scroll(self:click_scrollbar()) and
+        return true
-                self:getMousePos() and self.on_click then
+    end
-            self:on_click()
+    if keys._MOUSE_L_DOWN and self:getMousePos() and self.on_click then
-        end
+        self:on_click()
+        return true
     end
     if keys._MOUSE_R_DOWN and self:getMousePos() and self.on_rclick then
         self:on_rclick()
+        return true
     end
     for k,v in pairs(self.scroll_keys) do
-        if keys[k] then
+        if keys[k] and 0 ~= self:scroll(v) then
-            self:scroll(v)
+            return true
         end
     end
     return check_text_keys(self, keys)
@@ -871,7 +790,7 @@ end
 -- we can't set the text in init() since we may not yet have a frame that we
 -- can get wrapping bounds from.
 function WrappedLabel:postComputeFrame()
-    local wrapped_text = self:getWrappedText(self.frame_body.width)
+    local wrapped_text = self:getWrappedText(self.frame_body.width-1)
     if not wrapped_text then return end
     local text = {}
     for _,line in ipairs(wrapped_text:split(NEWLINE)) do
@@ -1107,7 +1026,11 @@ function List:postComputeFrame(body)
 end
 
 local function update_list_scrollbar(list)
-    self.scrollbar:update(list.page_top, list.page_size, #list.choices)
+    list.scrollbar:update(list.page_top, list.page_size, #list.choices)
+end
+
+function List:postUpdateLayout()
+    update_list_scrollbar(self)
 end
 
 function List:moveCursor(delta, force_cb)
@@ -1159,9 +1082,9 @@ end
 function List:on_scrollbar(scroll_spec)
     local v = 0
     if scroll_spec == 'down_large' then
-        v = math.floor(self.page_size / 2)
+        v = math.ceil(self.page_size / 2)
     elseif scroll_spec == 'up_large' then
-        v = -math.floor(self.page_size / 2)
+        v = -math.ceil(self.page_size / 2)
     elseif scroll_spec == 'down_small' then
         v = 1
     elseif scroll_spec == 'up_small' then

test/library/gui/widgets.Label.lua

@@ -11,70 +11,6 @@ fs.ATTRS = {
     focus_path = 'test-framed-screen',
 }
 
-function test.correct_frame_body_with_scroll_icons()
-    local t = {}
-    for i = 1, 12 do
-        t[#t+1] = tostring(i)
-        t[#t+1] = NEWLINE
-    end
-
-    function fs:init()
-        self:addviews{
-            widgets.Label{
-                view_id = 'text',
-                frame_inset = 0,
-                text = t,
-            },
-        }
-    end
-
-    local o = fs{}
-    expect.eq(o.subviews.text.frame_body.width, 9, "Label's frame_body.x2 and .width should be one smaller because of show_scrollbar.")
-end
-
-function test.correct_frame_body_with_few_text_lines()
-    local t = {}
-    for i = 1, 10 do
-        t[#t+1] = tostring(i)
-        t[#t+1] = NEWLINE
-    end
-
-    function fs:init()
-        self:addviews{
-            widgets.Label{
-                view_id = 'text',
-                frame_inset = 0,
-                text = t,
-            },
-        }
-    end
-
-    local o = fs{}
-    expect.eq(o.subviews.text.frame_body.width, 10, "Label's frame_body.x2 and .width should not change with show_scrollbar = false.")
-end
-
-function test.correct_frame_body_without_show_scrollbar()
-    local t = {}
-    for i = 1, 12 do
-        t[#t+1] = tostring(i)
-        t[#t+1] = NEWLINE
-    end
-
-    function fs:init()
-        self:addviews{
-            widgets.Label{
-                view_id = 'text',
-                frame_inset = 0,
-                text = t,
-                show_scrollbar = false,
-            },
-        }
-    end
-
-    local o = fs{}
-    expect.eq(o.subviews.text.frame_body.width, 10, "Label's frame_body.x2 and .width should not change with show_scrollbar = false.")
-end
-
 function test.scroll()
     local t = {}
     for i = 1, 12 do