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

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

Open
mzebrak opened this issue Oct 3, 2024 · 4 comments
Open

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

mzebrak opened this issue Oct 3, 2024 · 4 comments

Comments

@mzebrak
Copy link

mzebrak commented Oct 3, 2024

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
Copy link

github-actions bot commented Oct 3, 2024

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
Copy link
Collaborator

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
Copy link
Author

mzebrak commented Oct 3, 2024

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
Copy link
Author

mzebrak commented Oct 4, 2024

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 #5009 or #3127

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

No branches or pull requests

2 participants