Fyne: Consistent Selection API

[stuartmscott]

Is your feature request related to a problem? Please describe:

Selection APIs in widgets and collections are inconsistent.

Is it possible to construct a solution with the existing API?

n/a

Describe the solution you'd like to see:

Radio.ClearSelection()
Radio.SetSelectedOption(option string)
Radio.OnSelectionChanged(option string)

Select.ClearSelection()
Select.SelectedIndex() int
Select.SetSelectedOption(text string)
Select.SetSelectedIndex(index int)
Select.OnSelectionChanged(option string)

TabContainer.SelectedTab() *TabItem
TabContainer.SelectedIndex() int
TabContainer.SetSelectedTab(item *TabItem)
TabContainer.SetSelectedIndex(index int)
TabContainer.OnSelectionChanged(item *TabItem)

List.ClearSelection()
List.SelectedItem() int
List.SetSelectedItem(index int)
List.OnSelectionChanged(index int) // -1 == no selection

Table.ClearSelection()
Table.SelectedCell() (int, int)
Table.SetSelectedCell(row, col int)
Table.OnSelectionChanged(row, col int) // -1, -1 == no selection

Tree.ClearSelection()
Tree.SelectedNode() string
Tree.SetSelectedNode(uid string)
Tree.OnSelectionChanged(uid string) // tree.noSelection = "nothing" == no selection

Deprecation warning will be added in 1.4 and the APIs will be renamed in 2.0

[andydotxyz]

Radio and Select export the selection as Selected hence the setter is currently SetSelected. If we Change the setters we might need to change Field/Getter as well.

I think that we might find that using the same API for collection/container and simple input widgets may not be the simplest solution.

[stuartmscott]

As agreed in Slack, collections will use;

List.ClearSelection()
List.Selection() int
List.SetSelection(index int)
List.OnSelectionChanged(index int) // -1 == no selection

Table.ClearSelection()
Table.Selection() (int, int)
Table.SetSelection(row, col int)
Table.OnSelectionChanged(row, col int) // -1, -1 == no selection

Tree.ClearSelection()
Tree.Selection() string
Tree.SetSelection(uid string)
Tree.OnSelectionChanged(uid string) // tree.noSelection = "nothing" == no selection

[stuartmscott]

I think there is value in extending this convention to other widgets;

Radio.ClearSelection()
Radio.Selection() string
Radio.SelectionIndex() int
Radio.SetSelection(option string)
Radio.OnSelectionChanged(option string)

Select.ClearSelection()
Select.Selection() string
Select.SelectionIndex() int
Select.SetSelection(text string)
Select.SetSelectionByIndex(index int)
Select.OnSelectionChanged(option string)

TabContainer.Selection() *TabItem
TabContainer.SelectionIndex() int
TabContainer.SetSelection(item *TabItem)
TabContainer.SetSelectionByIndex(index int)
TabContainer.OnSelectionChanged(item *TabItem)

[andydotxyz]

Happy with the collection/container stuff.
Still a little unsure about Radio/Select as the exposed fields and nature of being input elements makes it a little less obvious that the changes fit. (They have exported Selected fields so Selection() getter would be weird).

[andydotxyz]

What other actions should we take on this ticket, should anything be moved out to another ticket?

[stuartmscott]

Let's move this over to 2.0 because I want to apply this consistency to AppTab and DocTab as well

[stuartmscott]

Including support for Multiple Selections has complicated the issue, however a Slack discussion has yielded a potential solution:

Collections by default will use single selection, and will have a field MultiSelect bool for the developer to enable multiple selection.

The selection API will use slices so they work in both single selection and multiple selection use cases;

list.OnSelectionChanged = func(indicies []int) {}
table.OnSelectionChanged = func(cells []Cell) {} // where type Cell struct {Row, Col int}
tree.OnSelectionChanged = func(uids []string) {}

In the above callbacks, developers can use if indicies|cells|uids == nil || len(indicies|cells|uids) == 0 to check whether the selection is empty.

The rest of the API will be;

// Get all current selections
func (l *List) Selection() []int
func (t *Table) Selection() []Cell
func (t *Tree) Selection() []string

// Clear all selections
func (l *List) ClearSelection()
func (t *Table) ClearSelection()
func (t *Tree) ClearSelection()

// Add selection to slice if MultiSelect, else replace slice with slice of single element (first parameter)
func (l *List) Select(int...)
func (t *Table) Select(Cell...)
func (t *Tree) Select(string...)

// Remove from selection slice
func (l *List) Unselect(int...)
func (t *Table) Unselect(Cell...)
func (t *Tree) Unselect(string...)

[stuartmscott]

Moving back to the 1.4 milestone so we can finalize the API before its first public release

[okratitan]

I really like this proposal and think it is the first one that has a clear way of consistently, and concisely I might add (Not 3-5 additional methods or 3 constants, or additional api), representing selection that works for every case we will have. Every other proposal so far has ended with debates with seemingly no solution or agreement for handling empty selection. This does it ideally with nil. Further any other proposal I've seen is going to carry technical debt by releasing apis that are not designed for multiple selection cases and A. subject to change in the near term OR B. Subject to be duplicated/made redundant.

[andydotxyz]

I worry that this seemingly overcomes the complexity of potential row/column selection in table by ignoring it. If the multiple selection is of Cell (which I think is missing from the illustration) then it seems like we’d need more APIs added to handle row/col which is where we started to see complexity using the previous model too

[okratitan]

func (t *Table) Selection() []Cell <--- multiple selection of cell. No APIs needed to handle row/col. Row and column selections shouldn't be passed or equated to being a selection value of table. They are convenience function for selecting all cells in a row or column. They themselves are not part of the value of what is being selected in a table. Clicking on a row or column and thus "selecting" it is selecting all of the cells in that row or column and that is what should be passed/managed in consistency. So func (t *Table) Selection() []Cell handles that case fine.

[andydotxyz]

I understand your position @okratitan that a row selection could be identified by just the cells that it contains, but implementing this way is limiiting us and many mainstream applications do not seem to handle selection like this, for example common spreadsheet apps refer to "E:E" for "all cells in column E" instead of "E1:E10000".

Additionally I would think carefully about the implications of using only cells to identify large selections, for example on a table that is 20'000x20'000 selecting a single row using row notation is 1 item of information to share, but using cells it is 20'000 items in the selection slice. If the user then selects 5 columns the col/row approach would have 5 data points to compare against whereas the cell based approach would be iterating through 100'000 items in the selection slice. This seems likely to become very slow without need.

[andydotxyz]

Thanks for the illustration @stuartmscott but I have concerns about designing and implementing an API for multi-selection in widgets that are not currently capable of this:

1) This could be rather confusing to developers - it is a singular data item using a slice notation
2) It will lead to many bug reports about multi selection not workiing
3) It is based on assumptions about how future multiple selection will work that we currently cannot test.

[okratitan]

Including support for Multiple Selections has complicated the issue, however a Slack discussion has yielded a potential solution:

Collections by default will use single selection, and will have a field MultiSelect bool for the developer to enable multiple selection.

The selection API will use slices so they work in both single selection and multiple selection use cases;

list.OnSelectionChanged = func(indicies []int) {}
table.OnSelectionChanged = func(cells []Cell) {} // where type Cell struct {Row, Col int}
tree.OnSelectionChanged = func(uids []string) {}

In the above callbacks, developers can use if indicies|cells|uids == nil || len(indicies|cells|uids) == 0 to check whether the selection is empty.

The rest of the API will be;

// Get all current selections
func (l *List) Selection() []int
func (t *Table) Selection() []Cell
func (t *Tree) Selection() []string

// Clear all selections
func (l *List) ClearSelection()
func (t *Table) ClearSelection()
func (t *Tree) ClearSelection()

// Add selection to slice if MultiSelect, else replace slice with slice of single element
func (l *List) Select(int)
func (t *Table) Select(Cell)
func (t *Tree) Select(string)

// Remove from selection slice
func (l *List) Unselect(int)
func (t *Table) Unselect(Cell)
func (t *Tree) Unselect(string)

I'm going to take a stab to update this for range support which I think should resolve a lot of other concerns:

Including support for Multiple Selections has complicated the issue, however a Slack discussion has yielded a potential solution:

Collections by default will use single selection, and will have a field MultiSelect bool for the developer to enable multiple selection.

The selection API will use slices so they work in both single selection and multiple selection use cases;

type ListRange struct {
        FirstIndex int,
        LastIndex int
}
type TableCell struct {
        Column int,
        Row int
}
type TableRange struct {
        FirstCell TableCell,
        LastCell TableCell
}

list.OnSelectionChanged = func(ranges []ListRange) {}
table.OnSelectionChanged = func(ranges []TableRange) {}
tree.OnSelectionChanged = func(uids []string) {}

In the above callbacks, developers can use ranges|uids == nil || len(ranges|uids) == 0 to check whether the selection is empty.

The rest of the API will be;

// Get all current selections
func (l *List) Selection() []ListRange
func (t *Table) Selection() []TableRange
func (t *Tree) Selection() []string

// Clear all selections
func (l *List) ClearSelection()
func (t *Table) ClearSelection()
func (t *Tree) ClearSelection()

// Add selection to slice if MultiSelect, else replace slice with slice of single element
func (l *List) Select(int)
func (t *Table) Select(TableCell)
func (t *Tree) Select(string)

// Remove from selection slice
func (l *List) Unselect(int)
func (t *Table) Unselect(TableCell)
func (t *Tree) Unselect(string)

[andydotxyz]

I don't want to copy for all of the widgets as above, I think we have established that they should be consistent. The crux of my confusion is abour how we don't have multiple selection but the recent proposals force this in the API somewhat prematurely.
I want to ensure that the callback types in selection match the callback types in the data handling, as they are referring to the same things. I would also like to see that we can elegantly handle row and column selection in table. Given that table seems to be the problematic one, here is a proposal for just that (list and tree would probably not adjust accordingly with minor name changes.

// Clear all
func (t *Table) ClearSelection()

// Select a cell - without multiple support this clears others
func (t *Table) Select(row, col int)

// get the current selection -- in a spreadsheet this is the cell that would be editable, even if other selections exist
func (t *Table) Selection() (row, col int)

// Unselect a single cell
func (t *Table) Unselect(row, col int)

/* then to add multiple support */

// turn on multiple selection in a table supporting it - off by default
Table.MultiSelection bool // or similar, that style matches Entry.MultiLine

// Get all current selections
func (t *Table) AllSelections() []TableSelection //initially just (row, col int) but we probably want range too

/* which could be used as so */

t.OnSelectionChanged = func(row, col int) {
  // could use just that for single - if we want to add row or col selections would be:
  if row == NoSelection {
    if col == NoSelection {
      // nothing selectted
    } else {
      // col selected 
    }
  } else {
    // row selected
  }
  // fall through to normal selection

  // OR if we want to know about multi we:
  all := list.AllSelections()
  if len(all) == 0 {
    // All unselected
  } else {
    // Selection made
    for _, sel := range all
  }
}

This supports a simple API for now, a match of data index API and a way to extend to multiple selections when we understand that requirement better.

[andydotxyz]

Following the call about this we have a new proposal that should satisfy the various requirements and has a plan for the future.

We managed to propose that these remain "simple" widgets so complexities like row / col headers, sorting and selection would be as part of an intermediate "presenter" that would manage data and some advanced features (assuming non-homogenous cells in table). So we were able to dismiss this from the collections widget APIs.

Single selection (1.4)

We introduce an ID type for each. Table benefits as it documents the two ints, tree benefits because we can express uniqueness of ID in just one place. And it's consistent

[Table]CellID is a struct of {int, int}
[List]ItemID is a typedef for int
[Tree]NodeID is a typedef for string

Each widget should be updated throughout to use these types for consistency (i.e. UpdatedCell(TableCellId))

For singular selection we expose On... for the selection and the unselection avoiding the need for zero check. Internally we can store as a slice for our benefit

Table.OnSelected func(TableCellID)
Table.OnUnselected func(TableCellID)

func (t *Table) Select(TableCellID) // first unselects existing, then selects new
func (t *Table) Unselect(TableCellID)

// then list
List.OnSelected func(ListItemID)
List.OnUnselected func(ListItemID)

func (l *List) Select(ListItemID)
func (l *List) Unselect(ListItemID)

// and tree
Tree.OnSelected func(TreeNodeID)
Tree.OnUnselected func(TreeNodeID)

func (t *Tree) Select(TreeNodeID) <- first unselects existing, then selects new
func (t *Tree) Unselect(TreeNodeID)

Moving to multi (1.5/2.0)

This is a simple implementation with no ranges and no optimisations

Add boolean Table.MultiSelect bool, defaults to false. Then we just add func (t *Table) SetSelection(TableCellID...) that iterates through existing selection to cancel them then iterates through the new ones to select.

List would have List.MultiSelect bool and func (l *List) SetSelection(ListItemID...), that iterates cancelling excisting, then selecting those passed.

Range

We could choose between the following options (but no impact on 1.4 or the multiselection above).
Probably no range support in Tree, until we can find a sensible way to express that it can only exist for notes with a common parent.

Range A, new exported types

A new range type, per collection, so we can have a Selection() []rangetype function.

TableRange type {start, end TableCellID}

Table.OnSelectedRange func(TableRange)
Table.OnUnselectedRange func(TableRange)

func (t *Table) SelectRange(TableRange) <- deselects any items/ranges inside this range, then adds to list or ranges
func (t *Table) UnselectRange(TableRange)

func (t *Table) Selection() []TableRange
func (t *Table) ClearSelection()
// and List
ListRange type {start, end ListItemD}

List.OnSelectedRange func(ListRange)
List.OnUnselectedRange func(ListRange)

func (l *List) SelectRange(ListRange) <- deselects any items/ranges inside this range, then adds to list or ranges
func (l *List) UnselectRange(ListRange)

func (l *List) Selection() []ListRange
func (l *List) ClearSelection()

Range B (no new exported type, no Selection())

Table.selection []tableRange // probably want an internal type for tracking

Table.OnRangeSelected func(start, end TableCellID)
Table.OnRangeUnselected func(start, end TableCellID)

func (t *Table) SelectRange(start, end TableCellID) <- deselects any single items inside this range, otherwise adds to list or ranges
func (t *Table) UnselectRange(start, end TableCellID)

// list
List.selection []listRange // probably want an internal type for tracking

List.OnRangeSelected func(start, end ListItemD)
List.OnRangeUnselected func(start, end ListItemD)

func (l *List) SelectRange(start, end ListItemD) <- deselects any single items inside this range, otherwise adds to list or ranges
func (l *List) UnselectRange(start, end ListItemD)

[stuartmscott]

Consider renaming ClearSelection to UnselectAll as this could be accompanied by SelectAll and has a nice symmetry with Select, Unselect, as well as SelectRange, UnselectRange.

[andydotxyz]

That seems like a good proposal. Keep the symettry whilst adding convenience once we support multi selection.
I think we are still good with the currrent proposal for 1.4 right?

[stuartmscott]

Single selection API landed in 1.4, moving to 2.0 milestone for multi selection