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

feat(console): help view modal #432

Merged
merged 2 commits into from
Jun 10, 2023
Merged

feat(console): help view modal #432

merged 2 commits into from
Jun 10, 2023

Conversation

hds
Copy link
Collaborator

@hds hds commented Jun 6, 2023

Adds a help modal which is available on every view. The help help modal
can be accessed by pressing ? and overlays the current view. To leave
the help modal, the user can press ? or Esc.

This PR is based on #243 originally authored by @bIgBV. The previous PR
has been dormant for around a year.

Currently the help modal only displays a vertical list of controls. This
is the same information that is available in the controls widget on each
view.

Here is an example of the tasks view with the help view modal active:

connection: http://localhost:6669/ (CONNECTED)
views: t = tasks, r = resources
controls: select column (sort) = ←→ or h, l, scroll = ↑↓ or k, j,
view details = ↵, invert sort (highest/lowest) = i, scroll to top = gg,
scroll to bottom╭Help──────────────────────────────────────────╮t = q
╭Warnings───────│controls:                                     │───────────────╮
│⚠ 1 tasks have │  select column (sort) = ←→ or h, l           │               │
╰───────────────│  scroll = ↑↓ or k, j                         │───────────────╯
╭Tasks (12) ▶ Ru│  view details = ↵                            │───────────────╮
│Warn  ID  State│  invert sort (highest/lowest) = i            │t      Location│
│       19 ▶    │  scroll to top = gg                          │::task console-│
│       22 ⏸    │  scroll to bottom = G                        │::task console-│
│⚠ 1    23 ⏸    │  toggle pause = space                        │::task console-│
│       24 ⏸    │  toggle help = ?                             │::task console-│
│       25 ⏸    │  quit = q                                    │::task console-│
│       74 ⏹    │                                              │::task console-│
│       75 ⏸    │                                              │::task console-│
│       77 ⏸    │                                              │::task console-│
│       78 ⏸    ╰──────────────────────────────────────────────╯::task console-│
│       79 ⏹      wait      11s    4ms   56µs    11s 2     tokio::task console-│
╰──────────────────────────────────────────────────────────────────────────────╯

However, the idea is that the help modal can provide contextual
information depending on the view and the state of the application being
observed. This will allow us to provide more details about any lints
which are currently triggering and also to reduce the height of the
current controls widget to just one line (perhaps optionally) as the
full list of controls can be accessed from the help view.

Co-authored-by: bIgBV bhargav.voleti93@gmail.com

Adds a help modal which is available on every view. The help help modal
can be accessed by pressing `?` and overlays the current view. To leave
the help modal, the user can press `?` or `Esc`.

This PR is based on #243 originally authored by @bIgBV. The previous PR
has been dormant for around a year.

Currently the help modal only displays a vertical list of controls. This
is the same information that is available in the controls widget on each
view.

Here is an example of the tasks view with the help view modal active:

```text
connection: http://localhost:6669/ (CONNECTED)
views: t = tasks, r = resources
controls: select column (sort) = ←→ or h, l, scroll = ↑↓ or k, j,
view details = ↵, invert sort (highest/lowest) = i, scroll to top = gg,
scroll to bottom╭Help──────────────────────────────────────────╮t = q
╭Warnings───────│controls:                                     │───────────────╮
│⚠ 1 tasks have │  select column (sort) = ←→ or h, l           │               │
╰───────────────│  scroll = ↑↓ or k, j                         │───────────────╯
╭Tasks (12) ▶ Ru│  view details = ↵                            │───────────────╮
│Warn  ID  State│  invert sort (highest/lowest) = i            │t      Location│
│       19 ▶    │  scroll to top = gg                          │::task console-│
│       22 ⏸    │  scroll to bottom = G                        │::task console-│
│⚠ 1    23 ⏸    │  toggle pause = space                        │::task console-│
│       24 ⏸    │  toggle help = ?                             │::task console-│
│       25 ⏸    │  quit = q                                    │::task console-│
│       74 ⏹    │                                              │::task console-│
│       75 ⏸    │                                              │::task console-│
│       77 ⏸    │                                              │::task console-│
│       78 ⏸    ╰──────────────────────────────────────────────╯::task console-│
│       79 ⏹      wait      11s    4ms   56µs    11s 2     tokio::task console-│
╰──────────────────────────────────────────────────────────────────────────────╯
```

However, the idea is that the help modal can provide contextual
information depending on the view and the state of the application being
observed. This will allow us to provide more details about any lints
which are currently triggering and also to reduce the height of the
current controls widget to just one line (perhaps optionally) as the
full list of controls can be accessed from the help view.

Co-authored-by: bIgBV <bhargav.voleti93@gmail.com>
@hds hds requested a review from a team as a code owner June 6, 2023 16:08
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

thanks for working on this! i had one suggestion for potentially simplifying the implementation a bit, but i'm fine with the current approach if you think it's better.

pub(crate) fn render<B: ratatui::backend::Backend>(
&mut self,
frame: &mut ratatui::terminal::Frame<B>,
area: layout::Rect,
state: &mut State,
) {
match self.state {
let help_text: &dyn HelpText = match self.state {
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: I'm not 100% convinced that the HelpText trait is really necessary here...couldn't we just have the help_text variable be a Paragraph, and have each view type have an inherent help_text() method that returns a Paragraph?

Copy link
Member

Choose a reason for hiding this comment

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

i guess the advantage of this approach is that we can assign help_text in the same match expression as rendering the view without having to actually allocate to build the paragraph if we're not displaying the help text. but, we could alternatively just put an additional match in the if self.show_help_modal block, which would call the current state's method to get a Paragraph of help text...

it's up to you whether this is actually better, i think the trait is fine, i just don't know if we really need it.

@hds
Copy link
Collaborator Author

hds commented Jun 9, 2023

My idea with the trait was to eventually do the same for the main view. It seemed like it would simplify things if we could return an object that implements MainView + HelpView or something like that and then render one or both depending on context.

I don't know if this is overengineering things though.

@hawkw
Copy link
Member

hawkw commented Jun 10, 2023

My idea with the trait was to eventually do the same for the main view. It seemed like it would simplify things if we could return an object that implements MainView + HelpView or something like that and then render one or both depending on context.

I don't know if this is overengineering things though.

That makes sense! If you're planning on continuing down this path in a subsequent PR, I think adding the HelpView trait now seems reasonable. If we try out a trait-object-based design and it doesn't work out, we can always change this code later, but I'm fine with merging this PR as-is.

@hawkw hawkw merged commit 5156e8e into main Jun 10, 2023
@hawkw hawkw deleted the hds/help-view branch June 10, 2023 13:45
hawkw pushed a commit that referenced this pull request Sep 29, 2023
hawkw added a commit that referenced this pull request Sep 29, 2023
# Changelog

All notable changes to this project will be documented in this file.
This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## tokio-console-v0.1.10 - (2023-09-29)

[7d009f8](https://github.com/tokio-rs/console/commit/7d009f87120ce0c89f5f9c5311f05b6756ca770f)...[7d009f8](https://github.com/tokio-rs/console/commit/7d009f87120ce0c89f5f9c5311f05b6756ca770f)

### <a id = "tokio-console-v0.1.10-breaking"></a>Breaking Changes
- **Update Tonic and Prost dependencies ([#364](#364 ([f9b8e03](https://github.com/tokio-rs/console/commit/f9b8e03bd7ee1d0edb441c94a93a350d5b06ed3b))<br />This commit updates the public dependencies `prost` and `tonic` to
semver-incompatible versions (v0.11.0 and v0.8.0, respectively). This is
a breaking change for users who are integrating the `console-api` protos
with their own `tonic` servers or clients.
- **Update `tonic` to v0.10 and increase MSRV to 1.64 ([#464](#464 ([96e62c8](https://github.com/tokio-rs/console/commit/96e62c83ef959569bb062dc8fee98fa2b2461e8d))<br />This is a breaking change for users of `console-api` and
`console-subscriber`, as it changes the public `tonic` dependency to a
semver-incompatible version. This breaks compatibility with `tonic`
0.9.x and `prost` 0.11.x.

### Added

- [**breaking**](#tokio-console-v0.1.10-breaking) Update Tonic and Prost dependencies ([#364](#364)) ([f9b8e03](f9b8e03))
- Only suggest opening issues for panics ([#365](#365)) ([da2a89c](da2a89c))
- Init error handling before subcmds ([#365](#365)) ([ec66eda](ec66eda))
- Filter out boring frames in backtraces ([#365](#365)) ([95a5e54](95a5e54))
- Include config options in autogenerated issues ([#365](#365)) ([3244a1f](3244a1f))
- Reduce decimal digits in UI ([#402](#402)) ([c13085e](c13085e))
- Use tokio task ids in task views ([#403](#403)) ([f5b06d2](f5b06d2))
- Add support for Unix domain sockets ([#388](#388)) ([a944dbc](a944dbc), closes [#296](#296))
- Add scheduled time per task ([#406](#406)) ([f280df9](f280df9))
- Add task scheduled times histogram ([#409](#409)) ([d92a399](d92a399))
- Update `tonic` to 0.9 ([#420](#420)) ([48af1ee](48af1ee))
- Update MSRV to Rust 1.60.0 ([b18ee47](b18ee47))
- Migrate to `ratatui` and update `crossterm` ([#425](#425)) ([b209dd6](b209dd6))
- Help view modal ([#432](#432)) ([359a4e7](359a4e7))
- Add way to inspect details of task from resource view ([#449](#449)) ([132ed4e](132ed4e), closes [#448](#448))
- Add warning for tasks that never yield ([#439](#439)) ([d05fa9e](d05fa9e))
- [**breaking**](#tokio-console-v0.1.10-breaking) Update `tonic` to v0.10 and increase MSRV to 1.64 ([#464](#464)) ([96e62c8](96e62c8))

### Documented

- Update screenshots in README ([#419](#419)) ([e9bcd67](e9bcd67))
- Revert "update screenshots in README ([#419](#419))" ([993a3d9](993a3d9))
- Update screenshots in README ([#421](#421)) ([8a27f96](8a27f96))
- Add column descriptions for all tables ([#431](#431)) ([e3cf82b](e3cf82b))
- Update MSRV version docs to 1.64 ([#467](#467)) ([94a5a51](94a5a51))

### Fixed

- Fix ascii-only flipped input ([#377](#377)) ([652ac34](652ac34))
- Declare `tokio-console` bin as `default-run` ([#379](#379)) ([9ce60ec](9ce60ec))
- Make `retain_for` default to 6s if not specfied ([#383](#383)) ([0a6012b](0a6012b), fixes [#382](#382))
- Enable view-switching keystrokes on details views ([#387](#387)) ([f417d7a](f417d7a))
- Fix `ViewOptions` default lang' ([#394](#394)) ([a1cf1b8](a1cf1b8), fixes [#393](#393))
- Remove `tracing-subscriber` 0.2 from dependencies ([#404](#404)) ([768534a](768534a))
- Fix calculation of busy time during poll ([#405](#405)) ([e2c536a](e2c536a))
- Remove histogram minimum count ([#424](#424)) ([02cf8a6](02cf8a6))
- Remove trailing space from task/resource location ([#443](#443)) ([90e5918](90e5918))
- Make long locations readable ([#441](#441)) ([9428d7f](9428d7f), closes [#411](#411))
- Fix task detail view Id to display remote tokio::task::Id ([#455](#455)) ([70c3952](70c3952))

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants