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

[feature] add help window feature when ? is pressed #70

Merged
merged 11 commits into from
Oct 27, 2024

Conversation

theghostmac
Copy link
Contributor

No description provided.

@radlinskii
Copy link
Owner

Hi, great job at adding the help window!

I have one suggestion, I think it should only be accessible Normal mode, so either during Pause, or before running the test.
Maybe also in the test results screen, but that can be a separate issue.

Guess the description wasn't clear enough, sorry about that.

@theghostmac
Copy link
Contributor Author

Hey @radlinskii, I just addressed that in the last 2 commits.

Nice project btw.

@theghostmac
Copy link
Contributor Author

Just did cargo fmt That should pass the build workflow.

@radlinskii
Copy link
Owner

the build job run is still failing, let me know if you plan to fix it yourself or not

@radlinskii
Copy link
Owner

ref #70

@theghostmac
Copy link
Contributor Author

I'll fix it. I've seen the issue.

@theghostmac
Copy link
Contributor Author

theghostmac commented Oct 26, 2024

Hi @radlinskii I've addressed the issue. Was having a hectic week at work, hence the delay.

  1. Changed the render method to take impl FrameWrapperInterface instead of Frame
  2. Removed the generic <B: Backend> since we're no longer using the raw Frame
  3. Changed direct frame methods to use the FrameWrapperInterface methods
  4. Simplified the imports since we don't need Backend anymore

@radlinskii
Copy link
Owner

radlinskii commented Oct 27, 2024

did you even try to run the app with your changes?

There is a runtime error when running the app on your branch, that is kind of hard to miss...

Before you've asked for approval even before the app would compile so I'm assuming you still didn't try to run it, which is kind of disturbing to me.

src/help_window.rs Outdated Show resolved Hide resolved
@radlinskii
Copy link
Owner

when help window is rendered then 's' keypress should not start test, or if it did, than the help window should hide.

@theghostmac
Copy link
Contributor Author

Okay, the most user-friendly one is the econd option -- to automatically hide help when 's' is pressed.

The user can now:

  • Toggle help with "?"
  • Press 's' to dismiss help and start the test in one action
  • Still use 'q' to quit whether help is shown or not

Any other changes required to improve the user experience?

@radlinskii radlinskii merged commit 4fcddd7 into radlinskii:main Oct 27, 2024
1 check passed
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