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

Rename UiVerbosity to UiLayout and make its variants more precise #6291

Merged
merged 5 commits into from May 13, 2024
Merged

Rename UiVerbosity to UiLayout and make its variants more precise #6291

merged 5 commits into from May 13, 2024

Conversation

abey79
Copy link
Contributor

@abey79 abey79 commented May 10, 2024 •
edited by github-actions bot

What

Pure refactor. The second commit clarify the meaning of the various UiLayout variants. The other commits are just renames.

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
  • If applicable, add a new check to the release checklist!

To run all checks from main, comment on the PR with @rerun-bot full-check.

@abey79 abey79 added 🚜 refactor Change the code, not the functionality exclude from changelog PRs with this won't show up in CHANGELOG.md labels May 10, 2024
@jleibs
Copy link
Member

jleibs commented May 10, 2024

UiContext seems too vague of a term... there's a lot of other ui-related context in other places such as ViewerContext::re_ui or egui::Context, and so just based on the name it's not immediately obvious to me that this context is just about density/verbosity of presentation.

A name more like RequestedLayout would make more sense to me.

@abey79
Copy link
Contributor Author

abey79 commented May 10, 2024

I agree. UiLayout maybe? Slightly shorter and better refers to both what's being requested and the "destination context" in which the ui will appear.

@abey79 abey79 changed the title Rename UiVerbosity to UiContext and make its variants more precise Rename UiVerbosity to UiLayout and make its variants more precise May 13, 2024
This was referenced May 13, 2024
Closed
Merged
@abey79 abey79 merged commit 24b1f18 into main May 13, 2024
34 of 35 checks passed
@abey79 abey79 deleted the antoine/ui-verbosity-rename branch May 13, 2024 11:55
abey79 added a commit that referenced this pull request May 14, 2024
@abey79
Improve UI of various components in the selection panel (#6297)
3ad7245 
### What

- Follow up to #6291 
- Fixes #6245 
- Unblocks #4161
- Limitation #6315

This PR fixes the `UiLayout::List` implementations of `DataUi` such that
they all fit on a single line and are deal with potentially narrow space
(mostly via truncation).

This PR unearthed quite some inconsistencies in how we're using
monospace font for data. For now, `text_ui` (which uses monospace) has
been renamed `data_label_for_ui_layout` to parallel the new
`label_for_ui_layout` (which uses proportional). In the future (#6315),
we must unify both with a better, flexible API that also allows mixed
styles.

TODO:
- [x] blob: check with large binaries -> OK (could certainly be improved
but it truncates correct 🤷🏻)
- [x] tensor data: fix ui for `size(shape) > 3`
- [x] range2d: improve label

### Screenshots

<img width="488" alt="image"
src="https://github.com/rerun-io/rerun/assets/49431240/e82c83c1-0f2b-4d64-9e76-71d28e3cba5c">
<img width="501" alt="image"
src="https://github.com/rerun-io/rerun/assets/49431240/da811427-cad2-463e-8285-9ae096441fc5">
<img width="369" alt="image"
src="https://github.com/rerun-io/rerun/assets/49431240/987af231-43c6-44e5-babb-52efdf110e30">
<img width="314" alt="image"
src="https://github.com/rerun-io/rerun/assets/49431240/2a4174bf-8117-46e5-9734-5e3ac05dd209">
<img width="231" alt="image"
src="https://github.com/rerun-io/rerun/assets/49431240/4fdc4e02-f6ff-43fc-8a68-163985ee0186">
<br/><br/>

When truncated:

<img width="228" alt="image"
src="https://github.com/rerun-io/rerun/assets/49431240/cbfbbb79-df18-48e9-be82-0401feec9d93">


### 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/6297?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/6297?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/6297)
- [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
Reviewers

@emilk emilk emilk approved these changes

Assignees
No one assigned
Labels
exclude from changelog PRs with this won't show up in CHANGELOG.md 🚜 refactor Change the code, not the functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@abey79 @jleibs @emilk