Skip to content
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

Implement table row framing and selection #3105

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

vvv
Copy link
Contributor

@vvv vvv commented Jun 22, 2023

  • Add framing effect to table rows
  • Select table row by clicking
  • Configure sensitivity of table cells

Demo

2023-06-29.01-12-25.mp4

Closes #1519, #3069

@vvv vvv force-pushed the table-row-select-and-frame branch from a520c38 to 6a8fe48 Compare June 26, 2023 13:52
@vvv
Copy link
Contributor Author

vvv commented Jun 26, 2023

cc @enomado, @max-huster (in case you still are interested in this functionality)

@vvv vvv force-pushed the table-row-select-and-frame branch from 6a8fe48 to 1c238f3 Compare June 26, 2023 19:56
@vvv vvv marked this pull request as draft July 3, 2023 09:29
@vvv
Copy link
Contributor Author

vvv commented Jul 3, 2023

💡 I have an idea of a simpler and more flexible implementation of row selection, that will not require as extensive changes to the library code.

New API

impl<'a> TableBuilder<'a> {
    /// Specify which row(s) are to be painted with [`Selection::bg_fill`][egui::style::Selection] background.
    pub fn select_rows(is_selected: impl Fn(usize) -> bool) -> Self {
        ...
    }
}

Row(s) selection mechanism

  • The application tells the library which rows to select by calling TableBuilder::select_rows.
  • Each of TableRow::cols (cells) returns a Response. The app checks the Responses to determine if any of this row's cells has been clicked.
  • The app has enough contextual information to deduce the index of the clicked row:

Comparison with the current design

Current implementation (this PR) only supports selecting a single row by storing its index in the table's IdTypeMap. New design allows users to store any number of row indices in the application's state, e.g.

pub struct App {
    selected_row: Option<usize>, // for a single row
}

or

pub struct App {
    selected_rows: Vec<usize>, // for multiple rows
}

The application handles mouse and keyboard events accordingly to its UI requirements. For example, the application can support the following interactions:

  • click on a row to select it
  • select a row by pressing Up/Down key
  • Cmd-click to add a row to selection
  • Shift-click to add a range of rows to the selection
  • Shift-Up/Down to extend/shrink the selection
  • Cmd-a to select all rows

@emilk
Copy link
Owner

emilk commented Aug 9, 2023

Nice, this is working really well!

I would suggest renaming the "framing effect" to something like "hover highlight" or similar. Seems more descritive.

@vvv
Copy link
Contributor Author

vvv commented Aug 9, 2023

Nice, this is working really well!

Thanks for your kind feedback, @emilk! 😊

I would suggest renaming the "framing effect" to something like "hover highlight" or similar. Seems more descritive.

Will do.

I don't think that we should merge this PR though. I have come up with a simpler and more flexible solution and marked this PR as draft.

I haven't implemented that other solution yet, I haven't even started.

I will find some hacking time this weekend. Please bear with me till Monday Aug 21st. I should have had plenty of time to finish by then.

Thanks to whoever is expecting this feature for your patience and words of support! And many thanks to @emilk for writing this wonderful piece of software!

@12089897411
Copy link

I'm really looking forward to this feature!

@vvv
Copy link
Contributor Author

vvv commented Sep 13, 2023

I will find some hacking time this weekend.

I am really sorry, folks.

I'm working my way into a stealth startup. And have another contract to deliver on.

I have no bandwidth left to implement this feature. 😞

@laurooyen
Copy link
Contributor

laurooyen commented Sep 13, 2023

I'll have a go at implementing this feature, including the newly proposed select_row api.

Edit: Expect a draft pull request by the end of the weekend. Deviated from the select_row api to make usage easier but maybe there's other solutions, feedback will be welcome.

@vvv
Copy link
Contributor Author

vvv commented Sep 13, 2023

@laurooyen That's great!

Do ping me if you need a sounding board or whatever little help I can provide.

Godspeed!

emilk added a commit that referenced this pull request Jan 6, 2024
* Based on #3105 by @vvv.

## Additions and Changes

- Add `TableBuilder::sense()` and `StripBuilder::sense()` to enable
detecting clicks or drags on table and strip cells.
- Add `TableRow::select()` which takes a boolean that sets the highlight
state for all cells added after a call to it. This allows highlighting
an entire row or specific cells.
- Add `TableRow::response()` which returns the union of the `Response`
of all cells added to the row up to that point. This makes it easy to
detect interactions with an entire row. See below for an alternative
design.
- Add `TableRow::index()` and `TableRow::col_index()` helpers.
- Remove explicit `row_index` from callback passed to
`TableBody::rows()` and `TableBody::heterogeneous_rows()`, possible due
to the above. This is a breaking change but makes the callback
compatible with `TableBody::row()`.
- Update Table example to demonstrate all of the above.

## Design Decisions

An alternative design to `TableRow::response()` would be to return the
row response from `TableBody`s `row()`, `rows()` and
`heterogeneous_rows()` functions. `row()` could just return the
response. `rows()` and `heterogeneous_rows()` could return a tuple of
the hovered row index and that rows response. I feel like this might be
the cleaner soluction if only returning the hovered rows response isn't
too limiting.

I didn't implement `TableBuilder::select_rows()` as described
[here](#3105 (comment))
because it requires an immutable borrow of the selection state for the
lifetime of the `TableBuilder`. This makes updating the selection state
from within the body unnecessarily complicated. Additionally the current
design allows for selecting specific cells, though that could be
possible by modifying `TableBuilder::select_rows()` to provide row and
column indices like below.

```rust
pub fn select_cells(is_selected: impl Fn(usize, usize) -> bool) -> Self
```

## Hover Highlighting

EDIT: Thanks to @samitbasu we now have hover highlighting too.

~This is not implemented yet. Ideally we'd have an api that allows to
choose between highlighting the hovered cell, column or row. Should
cells containing interactive widgets, be highlighted when hovering over
the widget or only when hovering over the cell itself? I'd like to
implement that before this gets merged though.~

Feedback is more than welcome. I'd be happy to make any changes
necessary to get this merged.

* Closes #1519
* Closes #1553
* Closes #3069

---------

Co-authored-by: Samit Basu <basu.samit@gmail.com>
Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants