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

Add framework to show warnings on screen #1231

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

cgzones
Copy link
Member

@cgzones cgzones commented Apr 23, 2023

Some interactive user action might fail, e.g. due to lacking
permissions. Those failures are currently hidden from the user. Add a
framework to register warning messages to be displayed for a pre-defined
amount of time on the screen.
Screenshot_20230423_200101

@cgzones cgzones force-pushed the warning branch 2 times, most recently from 5577088 to e9d61f9 Compare April 23, 2023 18:21
cgzones added 2 commits April 23, 2023 20:25
Some interactive user action might fail, e.g. due to lacking
permissions.  Those failures are currently hidden from the user.  Add a
framework to register warning messages to be displayed for a pre-defined
amount of time on the screen.
@BenBE
Copy link
Member

BenBE commented Apr 23, 2023

The actual change looks fine, but I'm not quite satisfied with the visual appeal of the warning messages as they somewhat break with the usual estetics of the UI. Why not have this as a Panel at the bottom above the status bar which only shows up there if there's something to show? Otherwise that panel simply hides away?

@Explorer09
Copy link
Contributor

Explorer09 commented Apr 24, 2023

A great proposal in general, but I have concern about the warning message showing as a "popup box".

It can bring some UX problems as htop is a terminal app, not a GUI app.

One problem I can think of is that it would be difficult to copy the entire warning message as plain text.

I would prefer that the warning is shown as a "bar" that spans a whole line on the terminal just above the function bar. I think my UI suggestion is mostly the same as BenBE's.

@Explorer09
Copy link
Contributor

Also, the warning message should allow an interactive way to dismiss. I.e. "press any key to continue"

If the warning message is triggered not by a user action, the message can be dismissed after a set number of seconds.
If the warning is triggered by a user action, then it should be dismissed only by a user action, so that user can always see the message.

@BenBE
Copy link
Member

BenBE commented Apr 24, 2023

I think the automatic dismiss as it is implemented right now is sufficient, given a good value for the timeout (didn't check, but 5-10 seconds should be fine).

What might be worth noting is handling of multiple warning messages: If there are multiple operations that failed, there should be a sensible way to notify about these without skipping over unrelated other messages (i.e. failures at startup).

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.

3 participants