Add option to set the screenshot path in `textual run`

[jessebot]

Context

I'm writing a tiny bash script to generate some screenshots from my textual app in a GitHub actions workflow. The action will run a few things like this:

textual run ./tui/base.py --screenshot 1
textual run ./tui/base.py --press n --screenshot 2
...

But screenshots are created with an autogenerated name like: baseapp_2023-12-02T13_22_06_312248.svg

I'd like to be able to set the name, and location if possible, of the screenshot so that I can put them into a directory called something like docs/screenshots and then have them referenced by my mkdocs markdown pages. This way, I can have a docs explicitly guiding users through each screen, which can help cover any gaps in accessibility I missed when trying to implement the screen reader and readable colors, like:

# welcome to my app
Here's an example of my welcome screen. You should see the word APP in all caps when 
you run my-app and then a text input box with the name of the thing you want to create.

<img src="./docs/screenshots/welcome.svg" alt="welcome screen for app showing the work APP and a text input box with no text in it">

Suggestion of how feature could work for end users

Example 1

Changing --screenshot to --screenshot-delay and adding --screenshot-name:

textual run ./tui/base.py --screenshot-delay 1 --screenshot-name welcome_screen.svg

which would generate a file called welcome_screen.svg that could be moved into the docs/screenshots directory.

You could also keep --screenshot the way it is and just add --screenshot-name

Example 2

Changing --screenshot to --screenshot-delay and adding --screenshot-path:

textual run ./tui/base.py --screenshot-delay 1 --screenshot-path ./docs/screenshots/welcome_screen.svg

Then the file gets written to ./docs/screenshots/welcome_screen.svg and a ci pipeline wouldn't need to do anything else except commit and push the file update.

You could also keep --screenshot the way it is and just add --screenshot-path

Additional info

My textual version and python version:

$ textual --version
textual, version 0.44.0

$ python --version
Python 3.11.2

I'm on an M1 mac pro running macOS 13.5.2.

Thanks to the textual team for all your hard work and help all the time, and sorry if I already missed this feature or a similar discussion or workaround. I tried to look, but didn't see one immediately popping out at me.

[github-actions[bot]]

We found the following entry in the FAQ which you may find helpful:

• How do I center a widget in a screen?

Feel free to close this issue if you found an answer in the FAQ. Otherwise, please give us a little time to review.

This is an automated reply, generated by FAQtory

[jessebot]

I've just realized I can probably just write a small python script instead of a bash script using the save_screenshot: https://textual.textualize.io/api/app/#textual.app.App.save_screenshot

But how do I press keys the way I would with the cli 🤔

[davep]

You may find https://blog.davep.org/2023/07/03/making-my-github-banner.html interesting,and keep in mind the pilot lets you press keys.

[jessebot]

Oh this is really cool! I will play with this more! Thank you! 🙏

It would still be cool to have the functionality in the cli, and if you want, I can attempt to work on it if you can point me to the place it would need to be done. I'm just always scared to try and contribute here because this repo is like what I look up to when it comes to Python. Incredible respect.

[jessebot]

Is there a fix for screenshots in browsers similar to the one listed here for terminals on macOS?: https://textual.textualize.io/FAQ/#why-doesnt-textual-look-good-on-macos

I generated the screenshot with:

from smol_k8s_lab.tui.base import BaseApp
import asyncio

async def make_base_screenshots() -> None:
    """
    make all the screenshots
    """
    async with BaseApp().run_test(size=(87, 47)) as pilot:
        # Test pressing the "tab" key followed by the "c" key
        await pilot.press("tab", "c")
        pilot.app.save_screenshot("./docs/images/screenshots/tui_config_screen.svg")
        
 if __name__ == "__main__":
    asyncio.run(make_base_screenshots())

All of mine look like that in screenshots as well, but I can't tell if its because my fonts are all bad because I used a bad fira code nerd font: https://github.com/small-hack/smol-k8s-lab/blob/clean-up-tui/docs/images/screenshots/tui_config_screen.svg

screenshots

Example sceenshot of the same terminal spacing issue: My font settings for firefox: It looks the same in safari, but I can't figure out how to check the font on safari 🤔

Other than that, I am rolling with the solution you posted in your blog, so thanks again very much! You can close this if you don't want this feature.

[davep]

I'll defer to @willmcgugan on if the suggested feature is something that would make sense (I don't think I've fully comprehended the suggestion myself; I just saw you mention screenshots and automated user interaction and thought "oh I sort of did that for a thing so my blog post might help". Also, if this is for docs, you may want to look at how the screenshots in our docs are made -- the code for them is run when the docs are built, keystrokes and all.

As for how the SVG looks: sadly I can't personally offer much help there, but I imagine it would be down to browser stuff. Your SVG looks fine to me in Chrome on macOS, if I view the SVG itself.

[willmcgugan]

Exposing the screenshot name seems like a easy win.

The screenshot SVGs work best when embedded. When in an img tag, the browser won't pull in the fonts. @jessebot if you have a look at our docs, you will see a link to the Roboto font which makes the screenshots look better.

That said, it's not perfect. The box characters don't line up perfectly. It's a limitation of the font and SVGs text rendering. There is a solution to this. We can replace all the box characters with paths which would ensure they fit together perfectly. I think we will do that eventually, but it is not a small job!

[rodrigogiraoserrao]

@jessebot are you interested in doing this (with some guidance) and do you have some time in the near future to do it?

[jessebot]

Yes and yes! :) I will have time later this week. I can start working on this on Wednesday I think, but I currently have a cold and can't think so good right now.

[rodrigogiraoserrao]

Yes and yes! :)

That's great! For now, how about we leave --screenshot DELAY untouched and just add --screenshot-path as the path to an SVG file where the screenshot is stored? Are you comfortable with creating a draft PR or do you need guidance with respect to the places where code needs to be changed?

I currently have a cold

I wish you a speedy recovery, in that case!

[jessebot]

I wish you a speedy recovery, in that case! Thank you! I just took a covid test and turns out it's actually covid. 😷 I was wondering why I've been sick since Friday 😮‍💨 It may be a couple of more days till I feel ok again. I'm still very brain foggy. I'm sorry :(

For now, how about we leave --screenshot DELAY untouched and just add --screenshot-path as the path to an SVG file where the screenshot is stored?

That sounds good! --screenshot-path is more than enough :)

Are you comfortable with creating a draft PR or do you need guidance with respect to the places where code needs to be changed?

I would need guidance, please. 🙏 I figured since it's a CLI it might be done with click or argparse, but "from click" and "import click" didn't return anything with a simple github search through this repo, and a search for "argparse" only returns the poetry lock. I didn't find this though, so I then realized maybe I'm in the wrong repo?

I then checked the textual-dev repo and found the cli stuff, I think: https://github.com/Textualize/textual-dev/blob/845e9e0f32b2ee827e8207a3e3ecf8738918147b/src/textual_dev/cli.py#L113-L119

I did find this in this repo?: https://github.com/Textualize/textual/blob/618a450b31e12f5dde011823bc47a4e7ead22e7f/src/textual/constants.py#L65 https://github.com/Textualize/textual/blob/618a450b31e12f5dde011823bc47a4e7ead22e7f/src/textual/app.py#L2318-L2326

Should I be submitting two PRs? One for to add the cli option, in textual-dev and one for this repo to update the above async def take_screenshot() to take a path for self.save_screenshot()? I can try working on both as soon as I feel less like I shouldn't be around production code, hopefully by this weekend?

Thank you for all your patience!

[rodrigogiraoserrao]

Take your time to take care of your health, @jessebot.

You are right, you'll need to add the CLI option to textual-dev and then make sure it gets used when running an app and taking a screenshot.

As you may start to understand from looking at the function textual-dev/cli.py::_run_app, we pass information to the app that will run via environment variables. That's being read in constants.py in the Textual side. We can do a similar thing for the screenshot path. Then, you can pass the path and filename to save_screenshot (if you look at the signature, you'll see it already accepts a path and a filename).

(Essentially, I'm confirming what you already said.)

In case you don't know, you can also use file paths as click argument types (docs). I'm not saying you need to use it, I'm just making sure you're aware that it exists.

[rodrigogiraoserrao]

Hey @jessebot, I hope you've recovered! Are you still interested in taking a look at this or should I remove your assignment?

[rodrigogiraoserrao]

Given that @jessebot is MIA I'll remove the assignment and move this back to TODO.

[github-actions[bot]]

Don't forget to star the repository!

Follow @textualizeio for Textual updates.

[jessebot]

Given that @jessebot is MIA I'll remove the assignment and move this back to TODO.

sorry about that! After covid some stuff happened with my personal life and I fell off of github for a while. Thank you to @davep for implementing this and sorry to the textual team for disappearing!

[davep]

No apology needed! Hope everything is heading back to okay.