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

Move re_ui UI functions to a trait to be implemented by egui::Ui #4569

Closed
abey79 opened this issue Dec 18, 2023 · 2 comments · Fixed by #6506
Closed

Move re_ui UI functions to a trait to be implemented by egui::Ui #4569

abey79 opened this issue Dec 18, 2023 · 2 comments · Fixed by #6506
Labels
💬 discussion enhancement New feature or request ui concerns graphical user interface
Milestone

Comments

@abey79
Copy link
Member

abey79 commented Dec 18, 2023

Most UI function in re_ui are #[allow(clippy::unused_self)], but they take a egui::Ui argument (obviously). This could be more cleanly implemented as a trait that would be implemented for egui::Ui.

Something like:

pub trait ReUi {
  fn ui(&mut self) -> &mut egui::Ui;

  fn small_icon_button(&self, icon: &Icon) -> egui::Response {
        ui.add(
            egui::ImageButton::new(icon.as_image().fit_to_exact_size(Self::small_icon_size()))
                .tint(self.ui().visuals().widgets.inactive.fg_stroke.color),
        )
    }

    // etc.
}

impl ReUi for egui::Ui {
  fn ui(&mut self) -> &mut egui::Ui { self }
}

Before (typical):

ctx.re_ui.small_icon_button(ui, icon);

After:

use re_ui::ReUi as _;

ui.small_icon_button(icon);
@abey79 abey79 added enhancement New feature or request 💬 discussion ui concerns graphical user interface labels Dec 18, 2023
@emilk
Copy link
Member

emilk commented Dec 18, 2023

be mindful that some of those functions are only NOT using self because they are using hard-coded values instead of the design tokens json

@emilk
Copy link
Member

emilk commented May 2, 2024

I'm getting tired of passing around ReUi everywhere, so perhaps it's time to do this.

One thing though: struct ReUi currently store the DesignTokens it uses in some of its member functions.
We would need to store that somewhere else, e.g. in egui's data store: ui.data(|data| data.get_temp::<Arc<DesignTokens>>())

@emilk emilk added this to the Triage milestone May 2, 2024
@emilk emilk assigned emilk and unassigned emilk May 17, 2024
abey79 added a commit that referenced this issue Jun 4, 2024
### What

- Fixes #6246
- Very tiny bit of #4569

This PR uses emilk/egui#4588 to replace the
`full_span` mechanism, which requires less boilerplate and makes it
available in many more places (including tooltips). The
`get_full_span()` function now lives in a `Ui` extension trait, where
the rest of `re_ui` should eventually migrate (#4569).

This PR also updates egui/egui_tiles/egui_commonmark to the latest
commits.

### 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/6491?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/6491?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/6491)
- [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 Jun 5, 2024
…traits and to `DesignTokens` (#6506)

### What

* Closes #4569 

This PR:
- Introduces `UiExt` and `ContextExt` extension traits for `egui::Ui`,
resp. `egui::Context`.
- Refactor `DesignTokens` such that it's globally available via
`re_ui::design_token()`.
- A new `re_ui::apply_style_and_install_loaders(&egui::Context)`
function is introduced to initialise Rerun style in egui.
- `ReUi` is removed, and it's methods are dispatched to `DesignTokens`,
`ContextExt`, and `UiExt`, depending on their nature.
- All `re_ui::ReUi` references are removed from the code base. Some
structures which formerly held a `ReUi` now hold a `egui::Context`
instead (including `App` and `ViewerContext`).
- Added some rules to `clippy.toml` to forbid the use of some `egui`
functions/type that are replaced by our own (e.g. `checkbox()` and
`radio_value()`). Cleaned up a redundant lint to that effect in
`scripts/lint.py`.

### 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/6506?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/6506?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/6506)
- [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
💬 discussion enhancement New feature or request ui concerns graphical user interface
Projects
None yet
2 participants