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.

Sign up for GitHub

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

feat(console): Add icon that sorting state #301

Merged
merged 10 commits into from
Mar 8, 2022
Merged

feat(console): Add icon that sorting state #301

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
4bc45d6
feat(subscriber): Add icon that sorting state
nrskt Mar 4, 2022
c29d6c9
feat(console): Support ASCII-only mode
nrskt Mar 4, 2022
762cacf
Merge branch 'main' into add_icon_asc_or_desc
hawkw Mar 4, 2022
ee30549
feat(console): Fix TableList trait
nrskt Mar 5, 2022
b16c853
feat(console): Add the methods for styling
nrskt Mar 5, 2022
b0e971f
feat(console): Fix the style of all tables
nrskt Mar 5, 2022
6bc5e6a
chore(console): Remove the unnecessary method.
nrskt Mar 5, 2022
9f660ec
Merge branch 'main' into add_icon_asc_or_desc
hawkw Mar 8, 2022
a648f5e
Update tokio-console/src/view/styles.rs
nrskt Mar 8, 2022
579c566
Merge branch 'main' into add_icon_asc_or_desc
hawkw Mar 8, 2022
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
4 changes: 4 additions & 0 deletions tokio-console/src/view/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,4 +228,8 @@ impl Width {
pub(crate) fn chars(&self) -> u16 {
self.curr
}

pub(crate) fn len(&self) -> usize {
self.curr as usize
}
}
24 changes: 20 additions & 4 deletions tokio-console/src/view/tasks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ impl TableList for TasksTable {
state: &mut State,
_: Self::Context,
) {
let state_len: u16 = Self::HEADER[2].len() as u16;
let mut state_len: u16 = Self::HEADER[2].len() as u16;
let now = if let Some(now) = state.last_updated_at() {
now
} else {
Expand Down Expand Up @@ -72,6 +72,18 @@ impl TableList for TasksTable {

let mut num_idle = 0;
let mut num_running = 0;

match table_list_state.sort_by {
SortBy::Warns => warn_width.update_len(warn_width.len() + 1),
SortBy::Tid => id_width.update_len(id_width.len() + 1),
SortBy::State => state_len += 1,
SortBy::Name => name_width.update_len(name_width.len() + 1),
SortBy::Polls => polls_width.update_len(polls_width.len() + 1),
SortBy::Target => target_width.update_len(target_width.len() + 1),
SortBy::Location => location_width.update_len(location_width.len() + 1),
SortBy::Total | SortBy::Busy | SortBy::Idle => (),
};
Copy link
Member

Choose a reason for hiding this comment

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

one note about this approach is that it means that changing the sorted column has the potential to make that column one character wider than it was previously, if none of the text in the column was longer than the header. i feel like this has the potential to look a little bit jarring if selecting a new column to sort by makes the columns shift around. it might honestly be better to just always add an additional character of width to every column?

but, i'm definitely willing to be convinced otherwise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i feel like this has the potential to look a little bit jarring if selecting a new column to sort by makes the columns shift around. it might honestly be better to just always add an additional character of width to every column?

I agree. I feel it would be more clear to add an additional character of width to every column. However, I am confused because there are a few places in the current implementation where the width is adjusted. I would like to fix it at another opportunity and understand the whole implementation a little bit.

Copy link
Member

Choose a reason for hiding this comment

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

We can always address that later!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I realize an idea.

  1. Add const WIDTHS param to TableList trait definition.
  2. Set the widths on the implementation.
pub(crate) trait TableList<const N: usize> {
    type Row;
    type Sort: SortBy + TryFrom<usize>;
    type Context;

    const HEADER: &'static [&'static str; N];
    const WIDTHS: &'static [usize; N];

    fn render<B: tui::backend::Backend>(
        state: &mut TableListState<Self, N>,
        styles: &view::Styles,
        frame: &mut tui::terminal::Frame<B>,
        area: layout::Rect,
        state: &mut state::State,
        cx: Self::Context,
    ) where
        Self: Sized;
}
impl TableList<11> for TasksTable {
    type Row = Task;
    type Sort = SortBy;
    type Context = ();

    const HEADER: &'static [&'static str; 11] = &[
        "Warn", "ID", "State", "Name", "Total", "Busy", "Idle", "Polls", "Target", "Location",
        "Fields",
    ];

    const WIDTHS: &'static [usize; 11] = &[
        Self::HEADER[0].len() + 1,
        Self::HEADER[1].len() + 1,
        Self::HEADER[2].len() + 1,
        Self::HEADER[3].len() + 1,
        Self::HEADER[4].len() + 1,
        Self::HEADER[5].len() + 1,
        Self::HEADER[6].len() + 1,
        Self::HEADER[7].len() + 1,
        Self::HEADER[8].len() + 1,
        Self::HEADER[9].len() + 1,
        Self::HEADER[10].len() + 1,
    ];

What do you think it?
(I worry about implement WIDTHS param is complex and boring)


let rows = {
let id_width = &mut id_width;
let target_width = &mut target_width;
Expand Down Expand Up @@ -149,11 +161,15 @@ impl TableList for TasksTable {
let header_style = header_style.add_modifier(style::Modifier::BOLD);

let header = Row::new(Self::HEADER.iter().enumerate().map(|(idx, &value)| {
let cell = Cell::from(value);
if idx == table_list_state.selected_column {
cell.style(selected_style)
let suffix = if table_list_state.sort_descending {
styles.if_utf8("▵", "+")
} else {
styles.if_utf8("▿", "-")
};
Cell::from(format!("{}{}", value, suffix)).style(selected_style)
} else {
cell
Cell::from(value)
}
}))
.height(1)
Expand Down