Merge pull request #370 from mwhudson/table-tweaks

Some tweaks to the table widget. Also build forms out of Tables.

subiquity/ui/views/filesystem/filesystem.py

@@ -48,7 +48,11 @@ from subiquitycore.ui.container import (
     )
 from subiquitycore.ui.form import Toggleable
 from subiquitycore.ui.stretchy import Stretchy
-from subiquitycore.ui.table import ColSpec, Table, TableRow
+from subiquitycore.ui.table import (
+    ColSpec,
+    TablePile,
+    TableRow,
+    )
 from subiquitycore.ui.utils import button_pile, Color, Padding, screen
 from subiquitycore.view import BaseView
 
@@ -178,7 +182,7 @@ class MountList(WidgetWrap):
 
     def __init__(self, parent):
         self.parent = parent
-        self.table = Table([], spacing=2, colspecs={
+        self.table = TablePile([], spacing=2, colspecs={
             0: ColSpec(can_shrink=True),
             1: ColSpec(min_width=9),
         })
@@ -264,7 +268,7 @@ class DeviceList(WidgetWrap):
     def __init__(self, parent, show_available):
         self.parent = parent
         self.show_available = show_available
-        self.table = Table([],  spacing=2, colspecs={
+        self.table = TablePile([],  spacing=2, colspecs={
             0: ColSpec(can_shrink=True),
             1: ColSpec(min_width=9),
         })

subiquity/ui/views/snaplist.py

@@ -33,7 +33,11 @@ from subiquitycore.ui.container import (
     ScrollBarListBox,
     WidgetWrap,
     )
-from subiquitycore.ui.table import ColSpec, Table, TableRow
+from subiquitycore.ui.table import (
+    AbstractTable,
+    ColSpec,
+    TableRow,
+    )
 from subiquitycore.ui.utils import button_pile, Color, screen
 from subiquitycore.view import BaseView
 
@@ -52,9 +56,11 @@ class StarRadioButton(RadioButton):
     reserve_columns = 3
 
 
-def NoTabCyclingListBox(body):
+class NoTabCyclingTableListBox(AbstractTable):
-    body = SimpleFocusListWalker(body)
+
-    return ScrollBarListBox(UrwidListBox(body))
+    def _make(self, rows):
+        body = SimpleFocusListWalker(rows)
+        return ScrollBarListBox(UrwidListBox(body))
 
 
 class SnapInfoView(WidgetWrap):
@@ -95,9 +101,7 @@ class SnapInfoView(WidgetWrap):
                 Text(notes),
             ])))
 
-        self.lb_channels = Table(
+        self.lb_channels = NoTabCyclingTableListBox(self.channels)
-            self.channels,
-            container_maker=NoTabCyclingListBox)
 
         title = Columns([
             Text(snap.name),
@@ -329,13 +333,12 @@ class SnapListView(BaseView):
                 Text(snap.summary, wrap='clip'),
                 ]
             body.append(Color.menu_button(TableRow(row)))
-        table = Table(
+        table = NoTabCyclingTableListBox(
             body,
             colspecs={
                 1: ColSpec(omittable=True),
-                2: ColSpec(can_shrink=True, min_width=40),
+                2: ColSpec(pack=False, min_width=40),
-                },
+                })
-            container_maker=NoTabCyclingListBox)
         ok = ok_btn(label=_("OK"), on_press=self.done)
         self._main_screen = screen(
             table, [ok],

subiquitycore/ui/form.py

@@ -28,13 +28,17 @@ from urwid import (
     )
 
 from subiquitycore.ui.buttons import cancel_btn, done_btn
-from subiquitycore.ui.container import Columns, Pile
 from subiquitycore.ui.interactive import (
     PasswordEditor,
     IntegerEditor,
     StringEditor,
     )
 from subiquitycore.ui.selector import Selector
+from subiquitycore.ui.table import (
+    ColSpec,
+    TablePile,
+    TableRow,
+    )
 from subiquitycore.ui.utils import (
     button_pile,
     Color,
@@ -104,6 +108,9 @@ class WantsToKnowFormField(object):
         self.bff = bff
 
 
+form_colspecs = {1: ColSpec(pack=False)}
+
+
 class BoundFormField(object):
 
     def __init__(self, field, form, widget):
@@ -116,26 +123,29 @@ class BoundFormField(object):
         self._help = None
         self.showing_extra = False
 
-        self._build_rows()
+        self._build_table()
 
         if 'change' in getattr(widget, 'signals', []):
             connect_signal(widget, 'change', self._change)
         if isinstance(widget, WantsToKnowFormField):
             widget.set_bound_form_field(self)
 
-    def _build_rows(self):
+    def _build_table(self):
         widget = self.widget
         if self.field.takes_default_style:
             widget = Color.string_input(widget)
-        validator = _Validator(self, widget)
 
         self.caption_text = Text(self.field.caption, align="right")
         self.under_text = Text(self.help)
 
-        row1 = Columns([self.caption_text, validator], dividechars=2)
+        self._rows = [
-        row2 = Columns([Text(""), self.under_text], dividechars=2)
+            Toggleable(TableRow(row)) for row in [
+                [self.caption_text, _Validator(self, widget)],
+                [Text(""),          self.under_text],
+                ]
+            ]
 
-        self._rows = Toggleable(Pile([row1, row2]))
+        self._table = TablePile(self._rows, spacing=2, colspecs=form_colspecs)
 
     def clean(self, value):
         cleaner = getattr(self.form, "clean_" + self.field.name, None)
@@ -224,12 +234,6 @@ class BoundFormField(object):
     def caption(self, val):
         self.caption_text.set_text(val)
 
-    def as_row(self, longest_caption):
-        for col, opt in self._rows.base_widget.contents:
-            col.contents[0] = (
-                col.contents[0][0], col.options('given', longest_caption))
-        return self._rows
-
     @property
     def enabled(self):
         return self._enabled
@@ -239,9 +243,11 @@ class BoundFormField(object):
         if val != self._enabled:
             self._enabled = val
             if val:
-                self._rows.enable()
+                for row in self._rows:
+                    row.enable()
             else:
-                self._rows.disable()
+                for row in self._rows:
+                    row.disable()
 
 
 def simple_field(widget_maker):
@@ -346,20 +352,16 @@ class Form(object, metaclass=MetaForm):
                 new_fields.append(bf)
         self._fields[:] = new_fields
 
-    @property
-    def longest_caption(self):
-        longest_caption = 0
-        for field in self._fields:
-            longest_caption = max(longest_caption, len(field.caption))
-        return longest_caption
-
     def as_rows(self):
-        longest_caption = self.longest_caption
+        if len(self._fields) == 0:
-        rows = []
+            return []
-        for field in self._fields:
+        t0 = self._fields[0]._table
-            rows.append(field.as_row(longest_caption))
+        rows = [t0]
+        for field in self._fields[1:]:
             rows.append(Text(""))
-        del rows[-1:]
+            t = field._table
+            t0.bind(t)
+            rows.append(t)
         return rows
 
     def as_screen(self, focus_buttons=True, excerpt=None):

subiquitycore/ui/table.py

@@ -19,11 +19,15 @@ A table widget.
 One of the principles of urwid is that widgets get their size from
 their container rather than deciding it for themselves. At times (as
 in stretchy.py) this does not make for the best UI. This module
-defines a Table widget that only takes up as much horizontal space as
+defines TablePile and TableListBox widgets that by default only take
-needed for its cells. If the table want more horizontal space than is
+up as much horizontal space as needed for their cells. If the table
-present, there is a degree of customization available as to what to
+wants more horizontal space than is present, there is a degree of
-do: you can tell which column to allow to shrink, and to omit another
+customization available as to what to do: you can tell which column to
-column to try to keep the shrinking column above a given threshold.
+allow to shrink, and to omit another column to try to keep the
+shrinking column above a given threshold.
+
+You can also let columns take all available space, as is the urwid
+default.
 
 Other features include cells that span multiple columns and binding
 tables together so that they use the same widths for their columns.
@@ -47,7 +51,7 @@ that have occurred to me during implementation:
 Example:
 
 ```
-    v = Table([
+    v = TablePile([
         TableRow([
             urwid.Text("aa"),
             (2, urwid.Text("0123456789"*5, wrap='clip')),
@@ -68,7 +72,12 @@ import logging
 
 
 from subiquitycore.ui.actionmenu import ActionMenu
-from subiquitycore.ui.container import Columns, Pile, WidgetWrap
+from subiquitycore.ui.container import (
+    Columns,
+    ListBox,
+    Pile,
+    WidgetWrap,
+    )
 
 import attr
 
@@ -81,17 +90,21 @@ log = logging.getLogger('subiquitycore.ui.table')
 @attr.s
 class ColSpec:
     """Details about a column."""
+    # Columns with pack=True take as much space as they need. Colunms
+    # with pack=False have the space remaining after pack=True columns
+    # are sized allocated to them.
+    pack = attr.ib(default=True)
     # can_shrink means that this column will be rendered narrower than
     # its natural width if there is not enough space for all columns
     # to have their natural width.
     can_shrink = attr.ib(default=False)
     # min_width is the minimum width that will be considered to be the
-    # columns natural width. If the column is shrinkable it might
+    # columns natural width. If the column is shrinkable (or
-    # still be rendered narrower than this.
+    # pack=False) it might still be rendered narrower than this.
     min_width = attr.ib(default=0)
     # omittable means that this column can be omitted in an effort to
-    # keep the width of a column with both can_shrink and min_width
+    # keep the width of a column with min_width set above that minimum
-    # set above that minimum width.
+    # width.
     omittable = attr.ib(default=False)
 
 
@@ -124,8 +137,7 @@ def widget_width(w):
                 r += widget_width(w1)
         r += (len(w.contents) - 1) * w.dividechars
         return r
-    else:
+    raise Exception("don't know how to find width of %r", w)
-        raise Exception("don't know how to find width of %r", w)
 
 
 class TableRow(WidgetWrap):
@@ -159,7 +171,7 @@ class TableRow(WidgetWrap):
             yield range(i, i+colspan), cell
             i += colspan
 
-    def get_natural_widths(self):
+    def get_natural_widths(self, unpacked_cols):
         """Return a mapping {column-index:natural-width}.
 
         Cells spanning multiple columns are ignored (handled in
@@ -167,17 +179,19 @@ class TableRow(WidgetWrap):
         """
         widths = {}
         for indices, cell in self._indices_cells():
-            if len(indices) == 1:
+            if len(indices) == 1 and indices[0] not in unpacked_cols:
                 widths[indices[0]] = widget_width(cell)
         return widths
 
-    def adjust_for_spanning_cells(self, widths, spacing):
+    def adjust_for_spanning_cells(self, unpacked_cols, widths, spacing):
         """Make sure columns are wide enough for cells with colspan > 1.
 
         This very roughly follows the approach in
         https://www.w3.org/TR/CSS2/tables.html#width-layout.
         """
         for indices, cell in self._indices_cells():
+            if set(indices) & unpacked_cols:
+                continue
             indices = [i for i in indices if widths[i] > 0]
             if len(indices) <= 1:
                 continue
@@ -190,7 +204,7 @@ class TableRow(WidgetWrap):
                 # whats needed.
                 div, mod = divmod(cell_width - cur_width, len(indices))
                 for i, j in enumerate(indices):
-                    widths[j] += div + int(i <= mod)
+                    widths[j] += div + int(i < mod)
 
     def set_widths(self, widths, spacing):
         """Configure row to given widths.
@@ -222,29 +236,33 @@ def _compute_widths_for_size(maxcol, table_rows, colspecs, spacing):
         ncols = sum(1 for w in widths.values() if w > 0)
         return sum(widths.values()) + (ncols-1)*spacing
 
+    unpacked_cols = {i for i, cs in colspecs.items() if not cs.pack}
+
     # Find the natural width for each column.
-    widths = {i: cs.min_width for i, cs in colspecs.items()}
+    widths = {i: cs.min_width for i, cs in colspecs.items() if cs.pack}
     for row in table_rows:
-        row_widths = row.base_widget.get_natural_widths()
+        row_widths = row.base_widget.get_natural_widths(unpacked_cols)
         for i, w in row_widths.items():
             widths[i] = max(w, widths.get(i, 0))
 
     # Make sure columns are big enough for cells that span mutiple
     # columns.
     for row in table_rows:
-        row.base_widget.adjust_for_spanning_cells(widths, spacing)
+        row.base_widget.adjust_for_spanning_cells(
+            unpacked_cols, widths, spacing)
 
-    # log.debug("%s %s %s", maxcol, widths, total(widths))
+    # log.debug("%s %s %s %s", maxcol, widths, total(widths), unpacked_cols)
 
     total_width = total(widths)
     # If there is not enough space, find a column that can shrink.
     #
     # If that column has a min_width, see if we need to omit any columns
     # to hit that target.
-    if total(widths) > maxcol:
+    if total_width > maxcol or unpacked_cols:
-        for i in list(widths):
+        for i in list(widths)+list(unpacked_cols):
-            if colspecs[i].can_shrink:
+            if colspecs[i].can_shrink or not colspecs[i].pack:
-                del widths[i]
+                if i in widths:
+                    del widths[i]
                 if colspecs[i].min_width:
                     while True:
                         remaining = maxcol - total(widths)
@@ -259,36 +277,26 @@ def _compute_widths_for_size(maxcol, table_rows, colspecs, spacing):
         total_width = maxcol
 
     # log.debug("widths %s", sorted(widths.items()))
-    return widths, total_width
+    return widths, total_width, bool(unpacked_cols)
 
 
-def default_container_maker(rows):
+class AbstractTable(WidgetWrap):
-    return Pile([('pack', r) for r in rows])
-
-
-class Table(WidgetWrap):
     # See the module docstring for docs.
 
-    def __init__(self, rows, colspecs=None, spacing=1,
+    def __init__(self, rows, colspecs=None, spacing=1):
-                 container_maker=default_container_maker):
         """Create a Table.
 
         `rows` - a list of possibly-decorated TableRows
         `colspecs` - a mapping {column-index:ColSpec}
         'spacing` - how much space to put between cells.
-
-        `container_maker` - something that makes a container out of a
-            sequences of rows. The default packs them all into a Pile,
-            the other option is to make a ListBox.
         """
         self.table_rows = [urwid.Padding(row) for row in rows]
         if colspecs is None:
             colspecs = {}
         self.colspecs = defaultdict(ColSpec, colspecs)
         self.spacing = spacing
-        self.container_maker = container_maker
 
-        super().__init__(container_maker(self.table_rows))
+        super().__init__(self._make(self.table_rows))
         self._last_size = None
         self.group = set([self])
 
@@ -298,7 +306,9 @@ class Table(WidgetWrap):
         Don't expect anything good to happen if the two tables do not
         use the same colspecs.
         """
-        self.group = other_table.group = self.group | other_table.group
+        new_group = self.group | other_table.group
+        for table in new_group:
+            table.group = new_group
 
     def _compute_widths_for_size(self, size):
         # Configure the table (and any bound tables) for the given size.
@@ -307,12 +317,13 @@ class Table(WidgetWrap):
         rows = []
         for table in self.group:
             rows.extend(table.table_rows)
-        widths, total_width = _compute_widths_for_size(
+        widths, total_width, has_unpacked = _compute_widths_for_size(
             size[0], rows, self.colspecs, self.spacing)
         for table in self.group:
             table._last_size = size
             for row in table.table_rows:
-                row.width = total_width
+                if not has_unpacked:
+                    row.width = total_width
                 row.base_widget.set_widths(widths, self.spacing)
 
     def rows(self, size, focus):
@@ -323,16 +334,27 @@ class Table(WidgetWrap):
         self._compute_widths_for_size(size)
         return super().render(size, focus)
 
-    def set_contents(self, rows):
+    @property
-        """Update the list of rows.
+    def focus_position(self):
+        return self._w.base_widget.focus_position
 
-        This might not work if container_maker makes a ListBox.
+    @focus_position.setter
-        """
+    def focus_position(self, val):
+        self._w.base_widget.focus_position = val
+
+
+class TablePile(AbstractTable):
+
+    def _make(self, rows):
+        return Pile([('pack', r) for r in rows])
+
+    def set_contents(self, rows):
+        """Update the list of rows. """
         self._last_size = None
         rows = [urwid.Padding(row) for row in rows]
         self.table_rows = rows
         empty_before = len(self._w.contents) == 0
-        self._w.contents[:] = self.container_maker(rows).contents
+        self._w.contents[:] = [(row, self._w.options('pack')) for row in rows]
         empty_after = len(self._w.contents) == 0
         # Pile / MonitoredFocusList have this strange behaviour where
         # when you add rows to an empty pile by assigning to contents,
@@ -342,10 +364,16 @@ class Table(WidgetWrap):
             self._select_first_selectable()
 
 
+class TableListBox(AbstractTable):
+
+    def _make(self, rows):
+        return ListBox(rows)
+
+
 if __name__ == '__main__':
     from subiquitycore.log import setup_logger
     setup_logger('.subiquity')
-    v = Table([
+    v = TablePile([
         TableRow([
             urwid.Text("aa"),
             (2, urwid.Text("0123456789"*5, wrap='clip')),