-
Notifications
You must be signed in to change notification settings - Fork 614
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Table #343
Closed
Closed
Table #343
Changes from all commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
18d9aab
add table example
bcampbell 3c8ada2
oops. table example now works.
bcampbell aef5f88
Merge remote-tracking branch 'upstream/table' into table
bcampbell 62ca6d1
add bare-bones table support for win32 (using comctl listview)
bcampbell fc5df3c
windows comctl table: notify listview upon model changes
bcampbell 24b7f3e
table example: switch to C++, generate dummy data
bcampbell b81111b
win32 comctl table: use std::vector instead of hand-rolled
bcampbell cff67d5
windows comctl table: tidy up WM_NOTIFY string hack
bcampbell 620bfd2
windows comctl table: win32 unexpectedly less bonkers than feared
bcampbell 067d605
windows comctl table: nicer default style
bcampbell 6547cc9
add my notes on table API and implementation
bcampbell 61e1cc2
table: add style flags and onSelectionChanged
bcampbell b5ed561
table (osx): support multiselect style flag
bcampbell de88440
refine table-selection notes across the platforms
bcampbell 5be3285
table: add access to current selection (gtk code)
bcampbell e13d11e
windows: support uiTable OnSelectionChanged callback
bcampbell 4870305
windows: uiTable support for selection iteration
bcampbell f75758e
table: move style flags into creation
bcampbell 11a98bf
mac: implement uiTableGetSelection and uiTableIter
bcampbell 0e56638
tables: add onSelectionChanged support to OSX version
bcampbell a9bacb2
table example: add delete button
bcampbell 6953267
Merge remote-tracking branch 'upstream/table' into table
bcampbell c32baa4
table: update ui[Free|Alloc] => uipriv[Free|Alloc]
bcampbell d70e977
update test for new uiNewTable
bcampbell 5f96010
apple-specific c++ build kludge for table example
bcampbell File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,106 @@ | ||
# Thoughts on table control support | ||
|
||
(BenC - 2017-09-25) | ||
|
||
## win32 comctl listview TODOs | ||
|
||
- maintain proper list of column parts, even if dummy. | ||
(or cut down the public API to text columns only for now | ||
until a better windows table control is available - see below | ||
for more thoughts on this) | ||
- support editing of cells | ||
- expandable parts (allocate leftover space) | ||
- how to determine column width? random sample of data? | ||
|
||
|
||
## Is the parts system the right abstraction? | ||
|
||
Some toolkits support implementing cell views as custom | ||
widget layouts eg [item delgates in QML](http://doc.qt.io/qt-5/qml-qtquick-controls-tableview.html#itemDelegate-prop). | ||
So an item can be just about any widget layout you want. | ||
|
||
Is that kind of thing a future aspiration for libui? | ||
If so, I'd suggest cutting the parts API back to just | ||
uiTableAppendTextColumn() for now. | ||
|
||
The parts API gives _some_ of the flexiblity, but would likely | ||
get in the way if libui went the whole hog on custom item view | ||
layouts. | ||
|
||
|
||
## API change: support bulk invalidation of model | ||
|
||
Currently, the only way to notify the control that new data has | ||
been added is via the uiTableModelRow[Inserted|Changed|Deleted] | ||
functions. This inevitably causes redraws for each item, which | ||
can get _really_ slow for large numbers of items. | ||
|
||
The use cases are: | ||
- adding large numbers of rows to existing models | ||
- sorting (eg clicking on various column headings to change | ||
the sort field and order. | ||
The user-supplied model needs to handle the sorting, the | ||
table control can't help. | ||
|
||
I think a single uiTableModelAllChanged() function would be | ||
enough. The underlying control needs to know that it should | ||
to redraw everything visible (and the number of items might | ||
have chnaged). | ||
|
||
|
||
## API change: support flags/styles | ||
|
||
- single or multiselect | ||
- hide column headers? | ||
- allow column reordering? | ||
- allow column resizing? | ||
- allow click on columns (+indicators for ascending/descending)? | ||
(will require corresponding event handling/callbacks) | ||
|
||
I'd suggest implementing whichever of these flags are supported on | ||
all platforms. | ||
|
||
|
||
|
||
|
||
## API Change: Selection | ||
|
||
In general, most toolkits avoid exposing a flat array of | ||
selected items, presumably because the dataset could be | ||
very large. Usually there are methods to iterate over | ||
selection, often with simplified interface for | ||
single-selection-only tables. | ||
|
||
[GtkTreeView](https://developer.gnome.org/gtk3/stable/GtkTreeView.html) | ||
- has a selection object for getting/setting selection | ||
- can fetch selection as a list, or via a foreach callback | ||
- need to map GtkTreeview paths back to row indexes | ||
|
||
[QML TableView](http://doc.qt.io/qt-5/qml-qtquick-controls-tableview.html) | ||
- has a selection object for getting/setting selection | ||
- supports iteration of selected items | ||
|
||
[OSX NSTableView](https://developer.apple.com/documentation/appkit/nstableview) | ||
- has an IndexSet to hold selected items | ||
- general purpose class? Lots of general set methods | ||
|
||
win32 commonctrl listview | ||
- items have LVIS_SELECTED flag | ||
- state changes notified via | ||
[LVN_ODSTATECHANGED](https://msdn.microsoft.com/en-us/library/windows/desktop/bb774859(v=vs.85).aspx) | ||
(indicates one or more contiguous items have changed state) | ||
- Use LVN_GETNEXTITEM to iterate through items with selected state | ||
|
||
[wxWidgets wxDataViewCtrl](http://docs.wxwidgets.org/3.0/classwx_data_view_ctrl.html) | ||
- provides `GetSelections( wxDataViewItemArray& sel)`, where `wxDataViewItemArray` | ||
is presumably a growable array of some kind. | ||
|
||
|
||
aiming to support: | ||
|
||
- a "selection has changed" event | ||
`uiTableOnSelectionChanged(uiTable* t, handlerfn fn, void* userdata)`? | ||
- functions to programatically select/unselect item(s) | ||
- access current selection (preferably by iteration rather than returning | ||
possibly-huge arrays) | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As noted, the problem is GTK+ does not seem to provide a way to do this. There might be workarounds that are inefficient (requiring a signal emission for every row of the table, for instance), and I'm not sure if removing and re-adding the model will reset other settings like column widths and scroll positions. Will need to ask the GTK+ people about it.
Might want to note this here. Not required; not part of review.