Textualize / textual

The lean application framework for Python. Build sophisticated user interfaces with a simple Python API. Run your apps in the terminal and a web browser.
https://textual.textualize.io/
MIT License
25.43k stars 780 forks source link

pop_until_active/get_screen can't work with "uninstalled" screens #5084

Open mzebrak opened 3 weeks ago

mzebrak commented 3 weeks ago

It's common that there's no need to pass a reference to some screen between multiple screens. This way when the desired screen is not "installed", you either have to access the screen stack directly to get the screen reference, or pass the reference.

There should be a better way to access screen instances stored in the stack by their type or by string.

This works:

from __future__ import annotations

from typing import TYPE_CHECKING

from textual.app import App, ComposeResult
from textual.screen import Screen
from textual.widgets import Label

if TYPE_CHECKING:
    from textual.pilot import Pilot

async def test_pop_until_active() -> None:
    class BaseScreen(Screen):
        def compose(self) -> ComposeResult:
            yield Label("BASE")

    class FooScreen(Screen):
        def compose(self) -> ComposeResult:
            yield Label("Foo")

    class BarScreen(Screen):
        BINDINGS = [("b", "app.make_base_active")]

        def compose(self) -> ComposeResult:
            yield Label("Bar")

    class PopApp(App):
        SCREENS = {"base": BaseScreen}

        async def on_mount(self) -> None:
            # Push base
            self.base_screen = BaseScreen(name="base")
            await self.push_screen(self.base_screen)
            # Push two screens
            await self.push_screen(FooScreen())
            await self.push_screen(BarScreen())

        def action_make_base_active(self) -> None:
            self.base_screen.pop_until_active()

    async with PopApp().run_test() as pilot:
        pilot: Pilot
        await pilot.press("b")
        assert isinstance(pilot.app.screen, BaseScreen)

But I see that there is no way to use pop_until_active when you do not store a reference to the screen when the screen is not "installed" also.

This doesn't work:

from __future__ import annotations

from typing import TYPE_CHECKING

from textual.app import App, ComposeResult
from textual.screen import Screen
from textual.widgets import Label

if TYPE_CHECKING:
    from textual.pilot import Pilot

async def test_pop_until_active() -> None:
    class BaseScreen(Screen):
        def compose(self) -> ComposeResult:
            yield Label("BASE")

    class FooScreen(Screen):
        def compose(self) -> ComposeResult:
            yield Label("Foo")

    class BarScreen(Screen):
        BINDINGS = [("b", "app.make_base_active")]

        def compose(self) -> ComposeResult:
            yield Label("Bar")

    class PopApp(App):
        SCREENS = {"base": BaseScreen}

        async def on_mount(self) -> None:
            # Push base
            await self.push_screen(BaseScreen())
            # Push two screens
            await self.push_screen(FooScreen())
            await self.push_screen(BarScreen())

        def action_make_base_active(self) -> None:
            self.get_screen("base").pop_until_active()

    async with PopApp().run_test() as pilot:
        pilot: Pilot
        await pilot.press("b")
        assert isinstance(pilot.app.screen, BaseScreen)

This is also not-so-good approach:

 self.screen_stack[-3].pop_until_active()

I think something like this should also work:

 self.get_screen(BaseScreen) # gets the screen from the screen stack by type, not only from installed screens
 self.get_screen("base") # because screen with such a name is defined in SCREENS, should be also able to find it when it's instance is pushed directly
github-actions[bot] commented 3 weeks ago

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

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

willmcgugan commented 3 weeks ago

As you can see from the docs, you refer to installed screens by their name. So do await self.push_screen("base")

Please read the guide on installed screens. First paragraph.

mzebrak commented 3 weeks ago

Yes, I know how to push the installed screen. But the issue is about the inability to pop to uninstalled screen as in the title.

Installed screen will be kept in memory and I don't want some screens to work like that.

Textual will keep these screens in memory for the lifetime of your app.

There's no way to get the screen instance from screen stack when it's not installed. Handy API like get_screen supports only installed screens.

Of course I could write some custom method based on public screen_stack, but wanted to point the issue with access to screen instances stored in the stack. Textual does not help much with that.

mzebrak commented 2 weeks ago

For anyone from the future - the "workaround" is to add a utility method to the App like:

    def get_screen_from_current_stack(self, screen: type[Screen[ScreenResultType]]) -> Screen[ScreenResultType]:
        for current_screen in self.screen_stack:
            if isinstance(current_screen, screen):
                return current_screen
        raise ScreenNotFoundError(f"Screen {screen} not found in stack")

then it can be used like:

self.app.get_screen_from_current_stack(SomeScreen).pop_until_active()

Still better than implementing own pop_until_active/pop_until_screen that I mentioned in https://github.com/Textualize/textual/issues/5009 or https://github.com/Textualize/textual/issues/3127