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

Remove ListItem clip-rect hack #5740

Closed
Tracked by #6075
emilk opened this issue Apr 2, 2024 · 0 comments · Fixed by #6161
Closed
Tracked by #6075

Remove ListItem clip-rect hack #5740

emilk opened this issue Apr 2, 2024 · 0 comments · Fixed by #6161
Labels
😤 annoying Something in the UI / SDK is annoying to use 🧑‍💻 dev experience developer experience (excluding CI) egui Requires egui/eframe work ui concerns graphical user interface

Comments

@emilk
Copy link
Member

emilk commented Apr 2, 2024

ListItem has a background that expands to fill the full width of the panel it is in.

This is implemented with an ugly hack, that makes it fill up to the clip_rect of whatever ui it is in, and counting on the clip_rect being correct for the panel or window.

This requires a lot of hacks in our code where we explicitly set the clip rect.

The problem is that the design is about filling the width of some ancestor Ui. It could be a panel, a Window, or a cell in a table, or something else.

Either we change the design, or come up with a better solution than using clip_rect… or we figure out a better way of enforcing this.

For instance, a popup Area/Window in egui has no clip rect, so when putting a ListItem in a tooltip, we need to explicitly set the clip_rect somewhere to something.

@emilk emilk added ui concerns graphical user interface 🧑‍💻 dev experience developer experience (excluding CI) 😤 annoying Something in the UI / SDK is annoying to use egui Requires egui/eframe work labels Apr 2, 2024
emilk added a commit that referenced this issue Apr 2, 2024
Requires a hack for the clip_rect.
I opened an issue to track this:
* #5740
emilk added a commit that referenced this issue Apr 2, 2024
Requires a hack for the clip_rect. I opened an issue to track this:
* #5740

Before:
![screenshot_2024-04-02_at_11 34
18](https://github.com/rerun-io/rerun/assets/1148717/8e4e55a2-4c97-4e4c-b504-061b56d7c624)

After:
<img width="847" alt="Screenshot 2024-04-02 at 11 59 16"
src="https://github.com/rerun-io/rerun/assets/1148717/e567f574-ed9c-469a-8d0c-34e344b8e7bb">


### 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 newly built examples:
[app.rerun.io](https://app.rerun.io/pr/5741/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/5741/index.html?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[app.rerun.io](https://app.rerun.io/pr/5741/index.html?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/5741)
- [Docs
preview](https://rerun.io/preview/04ae476663633878b1b60b6101d82e653454c0d2/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/04ae476663633878b1b60b6101d82e653454c0d2/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
abey79 added a commit that referenced this issue May 2, 2024
…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`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
😤 annoying Something in the UI / SDK is annoying to use 🧑‍💻 dev experience developer experience (excluding CI) egui Requires egui/eframe work ui concerns graphical user interface
Projects
None yet
1 participant