Skip to content

Commit

Permalink
[wicket] correctly wrap popup text within step logs (#3081)
Browse files Browse the repository at this point in the history
Previously, we were relying on tui's paragraph wrap functionality. This
had several limitations:

1. There's no way to obtain the actual height of the text as computed.
   This meant that wrapped text was previously cut off.
3. There's no sensible way to implement scrolling.

To address these issues, we implement wrapping ourselves, via the
textwrap library. The library exposes support for wrapping strings, but
we're working with tui Spans (which have styles associated with them) so
we have to do some work ourselves.  Thankfully, it isn't too much work:
textwrap exposes an abstract interface called `Fragment`, so it's mostly
a matter of copy-pasting some code from textwrap, and annotating each
word with its corresponding tui style.

This PR does not implement scrolling, but now that we're working with
pre-wrapped text in the popup, scrolling should be pretty simple to do
in a followup.

Screenshot of wrapped text:


![image](https://github.com/oxidecomputer/omicron/assets/180618/fb204b3a-3917-4e9d-bec8-4edb88c6cfdd)
  • Loading branch information
sunshowers authored May 11, 2023
1 parent 72b5757 commit 8b0ab46
Show file tree
Hide file tree
Showing 7 changed files with 441 additions and 8 deletions.
40 changes: 39 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@ tempdir = "0.3"
tempfile = "3.5"
term = "0.7"
termios = "0.3"
textwrap = "0.16.0"
test-strategy = "0.2.1"
thiserror = "1.0"
tofino = { git = "http://github.com/oxidecomputer/tofino", branch = "main" }
Expand All @@ -327,6 +328,7 @@ trybuild = "1.0.80"
tufaceous = { path = "tufaceous" }
tufaceous-lib = { path = "tufaceous-lib" }
tui = "0.19.0"
unicode-width = "0.1.10"
update-engine = { path = "update-engine" }
uuid = { version = "1.3.2", features = ["serde", "v4"] }
usdt = "0.3"
Expand Down
3 changes: 3 additions & 0 deletions wicket/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ futures.workspace = true
hex = { workspace = true, features = ["serde"] }
humantime.workspace = true
indexmap.workspace = true
itertools.workspace = true
omicron-common.workspace = true
once_cell.workspace = true
reqwest.workspace = true
Expand All @@ -32,11 +33,13 @@ slog-term.workspace = true
snafu.workspace = true
libsw.workspace = true
tar.workspace = true
textwrap.workspace = true
tokio = { workspace = true, features = ["full"] }
tokio-util.workspace = true
toml.workspace = true
tui.workspace = true
tui-tree-widget = "0.11.0"
unicode-width.workspace = true

update-engine.workspace = true
wicket-common.workspace = true
Expand Down
1 change: 1 addition & 0 deletions wicket/src/ui/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ mod main;
mod panes;
mod splash;
mod widgets;
mod wrap;

use crate::{Action, Cmd, State, Term};
use slog::{o, Logger};
Expand Down
12 changes: 11 additions & 1 deletion wicket/src/ui/panes/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use crate::ui::defaults::style;
use crate::ui::widgets::{
BoxConnector, BoxConnectorKind, ButtonText, IgnitionPopup, Popup,
};
use crate::ui::wrap::wrap_text;
use crate::{Action, Cmd, Frame, State};
use indexmap::IndexMap;
use omicron_common::api::internal::nexus::KnownArtifactKind;
Expand Down Expand Up @@ -342,9 +343,18 @@ impl UpdatePane {
}
}

// Wrap the text to the maximum popup width.
let options = crate::ui::wrap::Options {
width: Popup::max_content_width(state.screen_width) as usize,
initial_indent: Span::raw(""),
subsequent_indent: Span::raw(""),
break_words: true,
};
let wrapped_body = wrap_text(&body, options);

let popup = Popup {
header,
body,
body: wrapped_body,
buttons: vec![ButtonText {
instruction: "NAVIGATE",
key: "LEFT/RIGHT",
Expand Down
34 changes: 28 additions & 6 deletions wicket/src/ui/widgets/popup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use tui::{
layout::{Constraint, Direction, Layout, Rect},
style::Style,
text::{Span, Spans, Text},
widgets::{Block, BorderType, Borders, Clear, Paragraph, Widget, Wrap},
widgets::{Block, BorderType, Borders, Clear, Paragraph, Widget},
};

use crate::ui::defaults::dimensions::RectExt;
Expand Down Expand Up @@ -69,6 +69,28 @@ impl Popup<'_> {
});
u16::try_from(width).unwrap()
}

/// Returns the maximum width that this popup can have, including outer
/// borders.
///
/// This is currently 80% of screen width.
pub fn max_width(full_screen_width: u16) -> u16 {
(full_screen_width as u32 * 4 / 5) as u16
}

/// Returns the maximum width that this popup can have, not including outer
/// borders.
pub fn max_content_width(full_screen_width: u16) -> u16 {
Self::max_width(full_screen_width).saturating_sub(2)
}

/// Returns the maximum height that this popup can have, including outer
/// borders.
///
/// This is currently 80% of screen height.
pub fn max_height(full_screen_height: u16) -> u16 {
(full_screen_height as u32 * 4 / 5) as u16
}
}

impl Widget for Popup<'_> {
Expand All @@ -81,10 +103,9 @@ impl Widget for Popup<'_> {
.border_type(BorderType::Rounded)
.style(style::selected_line());

// Constrain width and height to 80% of screen width. Width and height
// are u16s and less than 128 so the computation shouldn't overflow.
let width = u16::min(self.width(), full_screen.width * 4 / 5);
let height = u16::min(self.height(), full_screen.height * 4 / 5);
let width = u16::min(self.width(), Self::max_width(full_screen.width));
let height =
u16::min(self.height(), Self::max_height(full_screen.height));

let rect =
full_screen.center_horizontally(width).center_vertically(height);
Expand Down Expand Up @@ -116,7 +137,8 @@ impl Widget for Popup<'_> {
.borders(Borders::BOTTOM | Borders::LEFT | Borders::RIGHT)
.render(chunks[1], buf);

let body = Paragraph::new(self.body).wrap(Wrap { trim: false });
// NOTE: wrapping should be performed externally, by e.g. wrap_text.
let body = Paragraph::new(self.body);

let mut body_rect = chunks[1];
// Ensure we're inside the outer border.
Expand Down
Loading

0 comments on commit 8b0ab46

Please sign in to comment.