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.

Sign up for GitHub

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

Jump to bottom

Tracking issue: ListItem 2.0 #6075

Open
5 of 11 tasks
abey79 opened this issue Apr 23, 2024 · 0 comments
Open
5 of 11 tasks

Tracking issue: ListItem 2.0 #6075

abey79 opened this issue Apr 23, 2024 · 0 comments
Assignees
@abey79
Labels
🎄 tracking issue issue that tracks a bunch of subissues ui concerns graphical user interface

Comments

@abey79
Copy link
Member

abey79 commented Apr 23, 2024 •
edited
Loading

@abey79 abey79 added ui concerns graphical user interface 🎄 tracking issue issue that tracks a bunch of subissues labels Apr 23, 2024
@abey79 abey79 self-assigned this Apr 23, 2024
This was referenced Apr 29, 2024
Closed
Merged
Merged
Merged
Merged
abey79 added a commit that referenced this issue May 2, 2024
@abey79
ListItem2.0 (part 1): introduce content-generic ListItem and `Lab…
f5cc5d7 
…elContent` legacy back-port (#6161)

### What

This PR does the following:
- Introduces the fundamental content-generic `ListItem` infrastructure
(`ListItem`, `trait ListItemContent`, `list_item_scope()`.
- Introduces `LabelContent`, a `ListItemContent` implementation which
implements the exact same features as the legacy `ListItem`.
- Updates `re_ui_example` to demonstrate the use of the new list item,
including a fairly extensive clean-up of the right panel code.

<img width="411" alt="image"
src="https://github.com/rerun-io/rerun/assets/49431240/dcd960d8-2fd1-48ed-ba5b-6f36bd35c65c">
 <br/> <br/>


- Part of #6075 
- Follow-up to #6148
- Fixes #5740

### Limitation and todos

- The handling of the X coordinate range for the background highlight
needs (introduced here to part with the clip rect hack) needs splitting
of to include _all_ full-span widgets: #6156.
- The state management currently looks meaningless as state will only be
used by the future `PropertyContent`. Funnily, all the state management
currently does is what is to be split off as per above :)
- Docstrings needs more work (in particular top-level overview)
- `ListItem` + `LabelContent` should be deployed wherever we currently
use `ListItem` 1.0, which should be then entirely removed.
- And of course, we need a two-column `PropertyContent`…

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6161?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6161?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/6161)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
abey79 added a commit that referenced this issue May 2, 2024
@abey79
ListItem 2.0 (part 2): introduce PropertyContent for two-column, …
9f6add2 
…property-like list items (#6174)

### What

This PR introduced the `PropertyContent`, an implementation of
`ListItemContent` for two-column list item, with a label and flexible
"values". Currently only demonstrated in `re_ui_example`.

What the "value" displays is delegated to a user-provided closure.
However, `PropertyContent` provides helper for a few basic types: bool,
text, color (both read-only and editable).

- Part of #6075
- Follow-up to #6161 


https://github.com/rerun-io/rerun/assets/49431240/bf94871a-63d5-46fa-94fe-a9adf720cdb8


### Limitations and todo

- Columns are fixed size at 50%. They will be made smart in [the next
PR.](#6182)
- More helpers are needed for various kinds of values.
- There can be only 0 or 1 action button. This should be extended by
using a `…` button with some kind of popup with all available actions in
a future PR.
- Right gutter space is reserved for the action button even if no list
item in scope use them. The `list_item_scope` could track this and skip
reserving that space if it's never used (e.g. component list in entity
path selection panel): #6179

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6174?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6174?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/6174)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
abey79 added a commit that referenced this issue May 2, 2024
@abey79
ListItem 2.0 (part 3): PropertyContent column auto-sizing (#6182)
d086044 
### What

This PR:
- Implements column auto-sizing for `PropertyContent`. The current
heuristics is rather simple and will likely need improvement when it's
battle tested. Currently, the content requests a minimum width of 200px
(there really isn't a way to properly deal with two-column-plus-button
neatly all the way to 0 width), and allocate a maximum of 70% of the
available width to the label column.
- Improves the way `ListItem` handles content capturing the hover state
(which happen for interactive content). There is no longer a requirement
for `ListItemContent::ui()` to return an `Option<Response>`, which
simplifies a bunch of things.
- Updates `re_ui_example` accordingly

Note: still no change applied to the actual viewer code base so far.

- Part of #6075 
- Follow-up to #6174

<img width="502" alt="image"
src="https://github.com/rerun-io/rerun/assets/49431240/28322870-997c-4bf4-997f-d0e7c8da2da9">

### Limitations and todo

- Improved heuristics for column sizing, when we have a better idea of
the need.
- More helpers are needed for various kinds of values.
- There can be only 0 or 1 action button. This should be extended by
using a `…` button with some kind of popup with all available actions in
a future PR.
- Right gutter space is reserved for the action button even if no list
item in scope use them. The `list_item_scope` could track this and skip
reserving that space if it's never used (e.g. component list in entity
path selection panel): #6179


### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6182?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6182?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/6182)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
abey79 added a commit that referenced this issue May 2, 2024
@abey79
ListItem 2.0 (part 4): only allocate space for property action butt…
9b7a0e8 
…ons when needed (#6183)

### What

☝🏻 

This PR also slightly adjust the semantics of the `id` parameter of
`list_item_scope()`. Now it just needs to be unique within the current
UI. It also addresses review comments from #6161, #6174, and #6182.

- Part of #6075
- Follow-up to #6182
- Fixes #6179


![Export-1714577757563](https://github.com/rerun-io/rerun/assets/49431240/45cb8284-15a9-4100-8057-bd7fd994d008)
<br/>

Note: the upper group of (nested) items (which makes use of action
buttons) has no impact on the lower group, because both sub-groups are
in a distinct `list_item_scope`.

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6183?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6183?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/6183)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
This was referenced May 2, 2024
Merged
Merged
abey79 added a commit that referenced this issue May 3, 2024
@abey79
ListItem 2.0 (part 5): deploy to the Visualizers and Overrides UIs (#…
0ac38a4 
…6184)

### What

This PR deploys `list_item2` to the visualizer and overrides UI
currently used for timeseries spaces views.

Unsurprisingly, contact with the real world required some adjustments:
- It's useful to be able to mix and match `LabelContent` and
`PropertyContent`. The following changes were needed for that to look
nice:
  - Both contents have now a configurable `min_desired_width`.
- `LabelContent` now has a `always_show_buttons` option that
perma-displays the buttons. This makes it easy to emulate
`PropertyContent`'s action button. This could be improved:
#6203
- Now the left column width is computed based on the total width
actually allocated by `ListItem` (as opposed to `ui.max_rect()` as seen
from `list_item_scope`). This is more correct when the list is in a
horizontal scroll area and fixes related visual glitches.

This PR also fixes the ShapeMarker editor UI, but required a workaround
due to `ui.max_rect()` behaving weird when the ComboBox menu expends due
to its contents.

- Part of #6075 
- Follow up to #6183 
- Fixes #4984
- Fixes #6205 

<img width="207" alt="image"
src="https://github.com/rerun-io/rerun/assets/49431240/60f85e2e-cc67-48a5-8331-e943c246453b">
<br/>



### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6184?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6184?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/6184)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
abey79 added a commit that referenced this issue May 6, 2024
@abey79
ListItem 2.0 (part 6): split full-span range management to a dedica…
0dfe602 
…ted module (#6211)

### What

So far, the full span x coordinate rang needed for highlighting was
maintained by `LayoutInfo` and `list_item_scope`. However, other widgets
also need this mechanism. This PR splits of full-span management to a
dedicated module, and deploys it both in `re_ui_example` and where
`list_item2` is currently used.

Other changes:
- This PR also improves the sanity checks regarding both
`list_item_scope` and `full_span_scope`. When missing, debug build will
panic and release build will emit warnings.
- Both scopes are now `ui.scope()`, so it's safe to modify `ui.style()`
from the closure.
- `list_item_scope` now sets `ui.spacing().item_spacing.y = 0`, since
this is required anyway for `ListItem`s to look good.

Follow-up PR will be needed to deploy `full_span` to all relevant
widgets and their use in the viewer (#6156).

- Part of #6075 
- Part of #6156
- Follow-up to #6184

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6211?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6211?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/6211)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
@abey79 abey79 pinned this issue May 7, 2024
@emilk emilk mentioned this issue Jun 25, 2024
Merged
5 tasks
abey79 pushed a commit that referenced this issue Jun 26, 2024
@emilk
Reorganize Selection Panel (#6637)
fd32e50 
### What
* Closes #6615
* Part of #6075

Each section now only does one thing, with a good name to describe it,
and in a good order.


![image](https://github.com/rerun-io/rerun/assets/1148717/a5f845cf-e0e4-496c-b495-85b7972c49f9)
![image](https://github.com/rerun-io/rerun/assets/1148717/b507f678-9139-4432-a2b6-c733ae0b0c94)


### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6637?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6637?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/6637)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
@abey79 abey79 unpinned this issue Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@abey79 abey79

Labels
🎄 tracking issue issue that tracks a bunch of subissues ui concerns graphical user interface
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@abey79