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

Single-selection drag-and-drop demos #4879

Merged
merged 37 commits into from
Jan 25, 2024
Merged

Single-selection drag-and-drop demos #4879

merged 37 commits into from
Jan 25, 2024

Conversation

abey79
Copy link
Member

@abey79 abey79 commented Jan 22, 2024

What

This PR implements a couple of single-selection drag-and-drop demo with ListItems in re_ui_example. One is flat, the other is hierarchical. Most of it happens in re_ui_example, with some minimal support for draggability in ListItem. The goal is to identify proper patterns (and egui TODOs) for drag-and-drop support in the blueprint tree UI.

TODO:

drag_and_drop_3.mp4

Contributes towards:

Checklist

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I have tested the web demo (if applicable):
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG

TODO
- figure out weird selection behaviour
- `ui.interact()` kills ListItem over -> migrate to `ListItem`
- clean logic code
TODO
- `ui.interact()` kills ListItem over -> migrate to `ListItem`
- clean logic code
@abey79 abey79 added ui concerns graphical user interface exclude from changelog PRs with this won't show up in CHANGELOG.md labels Jan 22, 2024
@abey79 abey79 changed the title Single-selection, non-hierarchical drag-and-drop demo Single-selection drag-and-drop demo Jan 22, 2024
@abey79 abey79 changed the title Single-selection drag-and-drop demo Single-selection drag-and-drop demos Jan 22, 2024
@abey79 abey79 marked this pull request as draft January 23, 2024 10:03
@abey79 abey79 marked this pull request as ready for review January 23, 2024 10:33
@emilk emilk self-requested a review January 23, 2024 11:17
Copy link
Member

@emilk emilk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can now rebase on main with new egui behavior and features, including response.contains_pointer

crates/re_ui/examples/re_ui_example.rs Outdated Show resolved Hide resolved
crates/re_ui/examples/re_ui_example.rs Outdated Show resolved Hide resolved
Comment on lines 1372 to 1374
insert_y: f32,
/// Range of X coordinates for the cursor
range_x: egui::Rangef,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this effectively forms a horizontal line - why not use a Rect here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because a rect is one more parameter than I need, and is thus ambiguous?

crates/re_ui/examples/re_ui_example.rs Outdated Show resolved Hide resolved
crates/re_ui/examples/re_ui_example.rs Outdated Show resolved Hide resolved
crates/re_ui/examples/re_ui_example.rs Outdated Show resolved Hide resolved
crates/re_ui/examples/re_ui_example.rs Outdated Show resolved Hide resolved
crates/re_ui/examples/re_ui_example.rs Outdated Show resolved Hide resolved
crates/re_ui/examples/re_ui_example.rs Outdated Show resolved Hide resolved
crates/re_ui/examples/re_ui_example.rs Show resolved Hide resolved
Copy link

github-actions bot commented Jan 23, 2024

Size changes

Name main 4879/merge Change
plots.rrd 194.56 kiB 204.80 kiB +5.26%

@abey79
Copy link
Member Author

abey79 commented Jan 23, 2024

We can now rebase on main with new egui behavior and features, including response.contains_pointer

Well funnily I finally don't need it in this case. I'm never testing for an "entire" response rect, but rather up to 4 sub-rects.

@abey79 abey79 requested a review from emilk January 23, 2024 16:26
emilk pushed a commit to emilk/egui that referenced this pull request Jan 24, 2024
This PR adds the following APIs, which I found to be missing while
working on rerun-io/rerun#4879:

- `Response::decidedly_dragged()`: tests if the corresponding widget is
being decidedly dragged
- `Memory::dragged_id()`: returns the ID of the dragged widget, if any
Copy link
Member

@emilk emilk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest moving the drag-and-drop demo to its own tab, or its own panel. I constantly tried to drag items to "Another section"

image

crates/re_ui/src/list_item.rs Outdated Show resolved Hide resolved
crates/re_ui/examples/re_ui_example.rs Outdated Show resolved Hide resolved
crates/re_ui/examples/re_ui_example.rs Outdated Show resolved Hide resolved
crates/re_ui/examples/re_ui_example.rs Outdated Show resolved Hide resolved
crates/re_ui/src/list_item.rs Show resolved Hide resolved
crates/re_ui/examples/re_ui_example.rs Outdated Show resolved Hide resolved
crates/re_ui/examples/re_ui_example.rs Outdated Show resolved Hide resolved
@emilk
Copy link
Member

emilk commented Jan 24, 2024

I miss the "I'm golding onto this item" effect that the egui.rs demo has:

drag-and-drop

Copy link
Member

@emilk emilk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heroic work this! ⭐

For extra style points, I suggest storing the DropTarget and interpolating it when it changes, just like drop-targets in egui_tiles. This makes for a much smoother experience for the user. But it can wait for a later PR :)

crates/re_ui/examples/re_ui_example.rs Outdated Show resolved Hide resolved
crates/re_ui/examples/re_ui_example.rs Outdated Show resolved Hide resolved
crates/re_ui/examples/re_ui_example.rs Outdated Show resolved Hide resolved
crates/re_ui/examples/re_ui_example.rs Outdated Show resolved Hide resolved
crates/re_ui/examples/re_ui_example.rs Outdated Show resolved Hide resolved
crates/re_ui/examples/re_ui_example.rs Outdated Show resolved Hide resolved
crates/re_ui/examples/re_ui_example.rs Show resolved Hide resolved
crates/re_ui/examples/re_ui_example.rs Show resolved Hide resolved
@abey79
Copy link
Member Author

abey79 commented Jan 24, 2024

I miss the "I'm [h]olding onto this item" effect that the egui.rs demo has.

and

For extra style points, I suggest storing the DropTarget and interpolating it when it changes, just like drop-targets in egui_tiles. This makes for a much smoother experience for the user. But it can wait for a later PR :)

I omitted those two things on purpose for two reasons:

  1. They become non-trivial as soon as multi-selection is involved.
  2. I'm sticking pretty closely to Figma as the "gold standard" of how to do these things.

If at all, I'm considering dragging a small "GH-label-like" blue bubble with the text "N items", as some added indication of what's being dragged, but that's for another PR (at least not until I do multi-selection dragging).

@abey79
Copy link
Member Author

abey79 commented Jan 24, 2024

I suggest moving the drag-and-drop demo to its own tab, or its own panel. I constantly tried to drag items to "Another section"

I've moved the demo to the top of the right panel and separated it more clearly from the rest.

@abey79 abey79 merged commit f720d38 into main Jan 25, 2024
40 checks passed
@abey79 abey79 deleted the antoine/dnd_demo branch January 25, 2024 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exclude from changelog PRs with this won't show up in CHANGELOG.md ui concerns graphical user interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants