Move row_top_offset to the TableDelegate
#14
Conversation
egui_table/src/table.rs
Outdated
| table_delegate: &dyn TableDelegate, | ||
| row_nr: u64, | ||
| ) -> f32 { | ||
| table_delegate.row_top_offset(ctx, self.id_salt, row_nr) |
There was a problem hiding this comment.
No, not the salt - the globally unique Id of the table (constructed from the salt and the parent ui id)
There was a problem hiding this comment.
OK.
Related (but I worked around it): I'm very confused by Table::get_id(). It's supposed to be the unique id for this table, but the returned result is going to be dependant on the Ui that is passed to it, right? Shouldn't the persistent id be generated once at the beginning of Table::show(), and then used throughout?
There was a problem hiding this comment.
If you see the test app you see we use get_id to access the state before calling .show. I'd love to have a better design for this, but not sure what it should be.
We could replace fn id_salt(salt impl Hash) with an fn id(id: Id), for instance. This is a recurring problem in egui, and one I haven't a uniform or nice solution for - yet.
There was a problem hiding this comment.
I see. Anyways, for the row offset stuff I could work around this in an acceptable way.
The initial implementation for arbitrary row height used a
row_top_offsetclosure pass theTablealongside the delegate. This approach is, however, not ideal. Often, the delegateneeds mutable access to data that is used to compute row height, thereby forcing the use of a lock of some kind.
This PR moves the
row_top_offsettoTableDelegate, with a default implementation which always returnsNone. Also,Table::row_heightis now renamedTable::default_row_height. The provided value is used whenrow_top_offset()is not implemented.Note:
row_top_offset()must always return eitherNone, orSome(offset). Returning inconsistantOptionvarient will lead to incoherent display.