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

PropertyContent: do not allocate space for acton button if no action button is ever used in the scope #6179

Closed
Tracked by #6075
abey79 opened this issue May 1, 2024 · 0 comments · Fixed by #6183
Closed
Tracked by #6075
Labels
ui concerns graphical user interface

Comments

@abey79
Copy link
Contributor

abey79 commented May 1, 2024

Currently, right-aligned gutter space is always reserved for action button in PropertyContent. It should only be reserved if at least one item in scope makes use of it. Otherwise it should be used for values.

@abey79 abey79 added the ui concerns graphical user interface label May 1, 2024
abey79 added a commit that referenced this issue May 2, 2024
…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
### 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
…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`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ui concerns graphical user interface
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant