Skip to content

Commit

Permalink
Change modal position to a fixed vertical distance from the top of th…
Browse files Browse the repository at this point in the history
…e window (#4700)

### What

This avoids the modal moving vertically when content size changes.

**NOTE**: this PR would trigger the bug fixed in
emilk/egui#3721, so it needs an egui release
before being released. This is avoided by salting the modals' ID such as
to in invalidate existing caches.


https://github.com/rerun-io/rerun/assets/49431240/d2d472b5-7af2-4e95-9da1-a671fc80629f


### 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/4700/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/4700/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/4700/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

- [PR Build Summary](https://build.rerun.io/pr/4700)
- [Docs
preview](https://rerun.io/preview/e159b005a7a1d5f44e608013f00c1d479b7c341c/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/e159b005a7a1d5f44e608013f00c1d479b7c341c/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
  • Loading branch information
abey79 authored Jan 5, 2024
1 parent 07bd6c7 commit a0d5946
Showing 1 changed file with 31 additions and 2 deletions.
33 changes: 31 additions & 2 deletions crates/re_ui/src/modal.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use egui::NumExt;

/// Helper object to handle a [`Modal`] window.
///
/// A [`Modal`] is typically held only so long as it is displayed, so it's typically stored in an
Expand Down Expand Up @@ -74,6 +76,25 @@ pub struct ModalResponse<R> {
/// [`Modal`] fakes as a modal window, since egui [doesn't have them yet](https://github.com/emilk/egui/issues/686).
/// This done by dimming the background and capturing clicks outside the window.
///
/// The positioning of the modal is as follows:
///
/// ```text
/// ┌─rerun window─────▲─────────────────────┐
/// │ │ 75px / 10% │
/// │ ╔═modal═▼══════════╗ ▲ │
/// │ ║ ▲ ║ │ │
/// │ ║ actual height │ ║ │ │
/// │ ║ based on │ ║ │ max │
/// │ ║ content │ ║ │ height│
/// │ ║ │ ║ │ │
/// │ ║ ▼ ║ │ │
/// │ ╚══════════════════╝ │ │
/// │ │ │ │ │
/// │ └───────▲──────────┘ ▼ │
/// │ │ 75px / 10% │
/// └──────────────────▼─────────────────────┘
/// ```
///
/// Note that [`Modal`] are typically used via the [`ModalHandler`] helper object to reduce boilerplate.
pub struct Modal {
title: String,
Expand Down Expand Up @@ -109,10 +130,18 @@ impl Modal {

let mut open = ui.input(|i| !i.key_pressed(egui::Key::Escape));

let screen_height = ui.ctx().screen_rect().height();
let modal_vertical_margins = (75.0).at_most(screen_height * 0.1);

let mut window = egui::Window::new(&self.title)
.pivot(egui::Align2::CENTER_CENTER)
.fixed_pos(ui.ctx().screen_rect().center())
//TODO(ab): workaround for https://github.com/emilk/egui/pull/3721 until we make a new egui release
.id(egui::Id::new(("modal", &self.title)))
.pivot(egui::Align2::CENTER_TOP)
.fixed_pos(
ui.ctx().screen_rect().center_top() + egui::vec2(0.0, modal_vertical_margins),
)
.constrain_to(ui.ctx().screen_rect())
.max_height(screen_height - 2.0 * modal_vertical_margins)
.collapsible(false)
.resizable(true)
.frame(egui::Frame {
Expand Down

0 comments on commit a0d5946

Please sign in to comment.