Skip to content
Closed
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
10 changes: 10 additions & 0 deletions tokio-console/src/input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,13 @@ pub(crate) fn is_space(input: &Event) -> bool {
})
)
}

pub(crate) fn is_help_toggle(event: &Event) -> bool {
matches!(
event,
Event::Key(KeyEvent {
code: KeyCode::Char('?'),
..
})
)
}
70 changes: 70 additions & 0 deletions tokio-console/src/view/help.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
use tui::{
layout::{self, Constraint, Direction, Layout},
text::Text,
widgets::{Clear, Paragraph, Wrap},
};

use crate::{state::State, view};

/// Simple view for help popup
pub(crate) struct HelpView<T> {
help_text: Option<T>,
}

impl<T> HelpView<T>
where
T: Into<Text<'static>>,
{
pub(super) fn new(help_text: T) -> Self {
HelpView {
help_text: Some(help_text),
}
}

pub(crate) fn render<B: tui::backend::Backend>(
&mut self,
styles: &view::Styles,
frame: &mut tui::terminal::Frame<B>,
_area: layout::Rect,
_state: &mut State,
Comment on lines +28 to +29
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a reason this has these arguments, when it doesn't use them? since this isn't a trait method, I think we should just not take these arguments just because they are passed to other views' render methods.

Suggested change
_area: layout::Rect,
_state: &mut State,

) {
let r = frame.size();
let content = self
.help_text
.take()
.expect("help_text should be initialized")
.into();
Comment on lines +32 to +36
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's kind of weird to me that help_text is an Option that we have to take here? Is there a reason the render method has to take an &mut self? It could just take self --- this isn't a trait method or anything.


let popup_layout = Layout::default()
.direction(Direction::Vertical)
.constraints(
[
Constraint::Percentage(40),
Constraint::Percentage(20),
Constraint::Percentage(40),
]
.as_ref(),
)
.split(r);

let popup_area = Layout::default()
.direction(Direction::Horizontal)
.constraints(
[
Constraint::Percentage(20),
Constraint::Percentage(60),
Constraint::Percentage(20),
]
.as_ref(),
)
.split(popup_layout[1])[1];

let display_text = Paragraph::new(content)
.block(styles.border_block().title("Help"))
.wrap(Wrap { trim: true });

// Clear the help block area and render the popup
frame.render_widget(Clear, popup_area);
frame.render_widget(display_text, popup_area);
}
}
33 changes: 33 additions & 0 deletions tokio-console/src/view/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::view::help::HelpView;
use crate::view::{resources::ResourcesTable, table::TableListState, tasks::TasksTable};
use crate::{input, state::State};
use std::{borrow::Cow, cmp};
Expand All @@ -8,15 +9,18 @@ use tui::{
};

mod async_ops;
mod help;
mod mini_histogram;
mod resource;
mod resources;
mod styles;
mod table;
mod task;
mod tasks;
use self::resource::ResourceView;
pub(crate) use self::styles::{Palette, Styles};
pub(crate) use self::table::SortBy;
use self::task::TaskView;

const DUR_LEN: usize = 10;
// This data is only updated every second, so it doesn't make a ton of
Expand All @@ -36,6 +40,7 @@ pub struct View {
tasks_list: TableListState<TasksTable, 11>,
resources_list: TableListState<ResourcesTable, 9>,
state: ViewState,
show_help_toggle: bool,
pub(crate) styles: Styles,
}

Expand Down Expand Up @@ -89,13 +94,17 @@ impl View {
state: ViewState::TasksList,
tasks_list: TableListState::<TasksTable, 11>::default(),
resources_list: TableListState::<ResourcesTable, 9>::default(),
show_help_toggle: false,
styles,
}
}

pub(crate) fn update_input(&mut self, event: input::Event, state: &State) -> UpdateKind {
use ViewState::*;
let mut update_kind = UpdateKind::Other;

self.handl_help_popup(event);

match self.state {
TasksList => {
// The enter key changes views, so handle here since we can
Expand Down Expand Up @@ -168,32 +177,56 @@ impl View {
update_kind
}

fn handl_help_popup(&mut self, event: crossterm::event::Event) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: should probably be

Suggested change
fn handl_help_popup(&mut self, event: crossterm::event::Event) {
fn handle_help_popup(&mut self, event: crossterm::event::Event) {

if input::is_help_toggle(&event) {
// TODO: Pause state if we are about to show the help toggle
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should probably do this before merging?

self.show_help_toggle = !self.show_help_toggle;
}
if self.show_help_toggle && !input::is_help_toggle(&event) {
Comment on lines +184 to +185
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, take it or leave it: if we used if ... else if, we could avoid the second call to input::is_help_toggle:

Suggested change
}
if self.show_help_toggle && !input::is_help_toggle(&event) {
} else if self.show_help_toggle {

// We have some other key pressed; clear the help popup.
self.show_help_toggle = !self.show_help_toggle;
}
}

pub(crate) fn render<B: tui::backend::Backend>(
&mut self,
frame: &mut tui::terminal::Frame<B>,
area: layout::Rect,
state: &mut State,
) {
let mut help_content;
match self.state {
ViewState::TasksList => {
self.tasks_list.render(&self.styles, frame, area, state, ());
help_content = HelpView::new(TableListState::<TasksTable>::render_help_content(
&self.styles,
));
}
ViewState::ResourcesList => {
self.resources_list
.render(&self.styles, frame, area, state, ());
help_content = HelpView::new(
TableListState::<ResourcesTable>::render_help_content(&self.styles),
);
}
ViewState::TaskInstance(ref mut view) => {
let now = state
.last_updated_at()
.expect("task view implies we've received an update");
view.render(&self.styles, frame, area, now);
help_content = HelpView::new(TaskView::render_help_content(&self.styles));
}
ViewState::ResourceInstance(ref mut view) => {
view.render(&self.styles, frame, area, state);
help_content = HelpView::new(ResourceView::render_help_content(&self.styles));
Comment on lines +201 to +221
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I notice that this will construct the help content for each view every time we render a screen, even if we're not showing the help window. That seems a little bit inefficient, since we have to allocate a big vec and a bunch of strings.

I think it might be more efficient to add a second match self.state inside of the if self.show_help_toggle box, and only build the help text if we are actually going to display a help box. That might make the method a bit longer, but I think it's worth doing to avoid having to format the help text when we're not going to display it...

}
}

state.retain_active();

if self.show_help_toggle {
help_content.render(&self.styles, frame, area, state);
}
}

pub(crate) fn current_view(&self) -> &ViewState {
Expand Down
4 changes: 4 additions & 0 deletions tokio-console/src/view/resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,4 +118,8 @@ impl ResourceView {
.render(styles, frame, async_ops_area, state, ctx);
self.initial_render = false;
}

pub(in crate::view) fn render_help_content(_styles: &view::Styles) -> Spans<'static> {
Spans::from(vec![Span::raw("A view to display help data for a resource")])
}
}
24 changes: 24 additions & 0 deletions tokio-console/src/view/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,30 @@ impl<T: TableList<N>, const N: usize> TableListState<T, N> {
) {
T::render(self, styles, frame, area, state, ctx)
}

pub(in crate::view) fn render_help_content(styles: &view::Styles) -> Spans<'static> {
Spans::from(vec![
Span::raw("controls: "),
bold(styles.if_utf8("\u{2190}\u{2192}", "left, right")),
Span::raw(" or "),
bold("h, l"),
text::Span::raw(" = select column (sort),\n"),
bold(styles.if_utf8("\u{2191}\u{2193}", "up, down")),
Span::raw(" or "),
bold("k, j"),
text::Span::raw(" = scroll, "),
bold(styles.if_utf8("\u{21B5}", "enter")),
text::Span::raw(" = view details, "),
bold("i"),
text::Span::raw(" = invert sort (highest/lowest), "),
bold("q"),
text::Span::raw(" = quit "),
bold("gg"),
text::Span::raw(" = scroll to top, "),
bold("G"),
text::Span::raw(" = scroll to bottom"),
])
}
}

impl<T, const N: usize> Default for TableListState<T, N>
Expand Down
4 changes: 4 additions & 0 deletions tokio-console/src/view/task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,10 @@ impl TaskView {
frame.render_widget(fields_widget, fields_area);
frame.render_widget(percentiles_widget, percentiles_area);
}

pub(in crate::view) fn render_help_content(_styles: &view::Styles) -> Spans<'static> {
Spans::from(vec![Span::raw("A view to diplay help data for a task")])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this also include controls?

}
}

impl Details {
Expand Down