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

[wicket] correctly wrap popup text within step logs #3081

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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