Different behaviour between CSS and CSS_PATH of Screen

[mzebrak]

Consider such an MRE:

from __future__ import annotations

from textual.app import App, ComposeResult
from textual.binding import Binding
from textual.screen import Screen
from textual.widgets import Button, Footer


class CssScreen(Screen):
    CSS = """
    Button {
        color: green;
    }
    """

    BINDINGS = [
        Binding("f1", "pop_screen", "Pop Screen"),
    ]

    def compose(self) -> ComposeResult:
        yield Button("This and only this should be GREEN")
        yield Footer()


class CssPathScreen(Screen):
    CSS_PATH = ["./textual_css_leaking.scss"]

    BINDINGS = [
        Binding("f1", "pop_screen", "Pop Screen"),
    ]

    def compose(self) -> ComposeResult:
        yield Button("This and only this should be RED")
        yield Footer()


class MainScreen(Screen):
    BINDINGS = [
        Binding("f1", "push_css_screen", "Push CSS Screen"),
        Binding("f2", "push_css_path_screen", "Push CSS Path Screen"),
    ]

    def compose(self) -> ComposeResult:
        yield Button("This and only this should be GRAY (default)")
        yield Footer()

    def action_push_css_screen(self) -> None:
        self.app.push_screen(CssScreen())

    def action_push_css_path_screen(self) -> None:
        self.app.push_screen(CssPathScreen())


class CssLeakingApp(App):

    def on_mount(self):
        self.push_screen(MainScreen())


CssLeakingApp().run()

and textual_css_leaking.scss:

Button {
    background: red;
}

When going from CssScreen back to MainScreen - it looks like expected behavior for me, because the button defined in MainScreen doesn't change. But when going back from CssPathScreen, MainScreen button has also its background set to red.

So this clearly shows CSS applies only to the current screen, but CSS_PATH applies to the whole app.
That's weird because in the docs of screen we can read:

that both should apply to the whole app.

But IMO this behavior is wrong, and such a CSS or CSS_PATH defined on the Screen or also Widget level should apply only to that screen/widget and not the whole app. For the whole app, we already have CSS and CSS_PATH in the App.

Of course we can workaround this by making it like

CssPathScreen Button {
    background: red;
}

but I think this issue should be addressed, most of the times you want to only change CSS of the current screen/widget, and of course, to the whole app you have CSS and CSS_PATH in App.

This would be also consistent with:

Breaking change: CSS in DEFAULT_CSS is now automatically scoped to the widget (set SCOPED_CSS=False) to disable

introduced in 0.38.0

---

Also I think maybe Widget also should have CSS? because now its CSS can be only modified by DEFAULT_CSS (which I think is rather intended for textual internal styling). That already showed an issue here, the solution was to use CSS over DEFAULT_CSS. For Widget it would be a problem because it only has DEFAULT_CSS.

And another thing with Widget CSS, IMO related: #3823

[davep]

I suspect this may be a documentation issue more than anything. What you seem to be expecting is how things used to work until scoped CSS was introduced. You can still go back to the old behaviour with the SCOPED_CSS setting. For example, compare this:

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

class ChildScreen(ModalScreen[None]):

    CSS = """
    ChildScreen {
        align: center middle;
    }

    Label {
        background: red;
    }
    """

    BINDINGS = [
        ("s", "close"),
    ]

    def compose(self) -> ComposeResult:
        yield Label("This label is on the child screen, composed from the child screen")

    def action_close(self) -> None:
        self.app.pop_screen()

class ScreenCSSApp(App[None]):

    BINDINGS = [
        ("s", "show_child"),
    ]

    def compose(self) -> ComposeResult:
        yield Label("This label is on the default screen, composed from the App")

    def action_show_child(self) -> None:
        self.push_screen(ChildScreen())

if __name__ == "__main__":
    ScreenCSSApp().run()

vs this:

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

class ChildScreen(ModalScreen[None]):

    SCOPED_CSS = False

    CSS = """
    ChildScreen {
        align: center middle;
    }

    Label {
        background: red;
    }
    """

    BINDINGS = [
        ("s", "close"),
    ]

    def compose(self) -> ComposeResult:
        yield Label("This label is on the child screen, composed from the child screen")

    def action_close(self) -> None:
        self.app.pop_screen()

class ScreenCSSApp(App[None]):

    BINDINGS = [
        ("s", "show_child"),
    ]

    def compose(self) -> ComposeResult:
        yield Label("This label is on the default screen, composed from the App")

    def action_show_child(self) -> None:
        self.push_screen(ChildScreen())

if __name__ == "__main__":
    ScreenCSSApp().run()

In other words: you should consider the CSS of a screen to be scoped to that screen unless you tell it otherwise.

[mzebrak]

In other words: you should consider the CSS of a screen to be scoped to that screen unless you tell it otherwise.

I want a little bit different thing - CSS_PATH to be scoped only to that screen. The default behavior of CSS (to have limited scope is very good choice IMO). The same should be true for CSS_PATH... (so exactly the same behavior will be as default when using CSS and CSS_PATH)

I also mentioned other related problems with Widget class in this issue.

[davep]

I want a little bit different thing - CSS_PATH to be scoped only to that screen. The default behavior of CSS (to have limited scope is very good choice IMO). The same should be true for CSS_PATH...

I believe that's an intended enhancement at some point, see the conversation in #2744.

I also mentioned other related problems with Widget class in this issue.

I hope you appreciate that it isn't incumbent on someone providing information on one issue to cover all the issues; moreover, if you have multiple issues, it's a good idea to report them as multiple issues.

[mzebrak]

I believe that's an intended enhancement at some point, see the conversation in #2744.

Thanks for that. Will read in a moment.

It's a pity that you didn't mention it earlier, but you just reduced this problem to "typo in the documentation".

I would like to highlight a problem which, in my opinion, is quite important because it is misleading (apart from the documentation itself) - at least in my opinion it is expected that if you set CSS, it doesn't matter whether you hardcode it in the widget (which I don't like) or point to the file (which is better because it also allows live-reloading), it should behave the same.

That's why I'm creating an issue and I would expect some discussion over the actual problem instead of a simple attempt to close the problem with a trivial solution when the real problem is still not explained.

For example, I would like to know what you think so that I can think about my code in a more future-oriented way and find a solution to the problem that won't fall apart in a moment. The fact that you linked the thread gives me hope that it will be done as I expected. However the initial reduction of it to a "documentation error" suggested that no change was intended in the direction I expected.

[davep]

It's a pity that you didn't mention it earlier, but you just reduced this problem to "typo in the documentation".

I have no idea what this means. I linked to a related PR the moment I better understood what you were looking for, after you clarified, and at no point have I said anything about typos.

That's why I'm creating an issue and I would expect some discussion over the actual problem instead of a simple attempt to close the problem with a trivial solution when the real problem is still not explained.

I fail to see how adding context is "a simple attempt to close the problem". Issues are either open or closed, there is no visible "attempt".

I sense my help here isn't welcome; I'll leave this for someone else to pick up.

[mzebrak]

It's a pity that you didn't mention it earlier, but you just reduced this problem to "typo in the documentation".

I have no idea what this means. I linked to a related PR the moment I better understood what you were looking for, after you clarified, and at no point have I said anything about typos.

That's why I'm creating an issue and I would expect some discussion over the actual problem instead of a simple attempt to close the problem with a trivial solution when the real problem is still not explained.

I fail to see how adding context is "a simple attempt to close the problem". Issues are either open or closed, there is no visible "attempt".

I sense my help here isn't welcome; I'll leave this for someone else to pick up.

It seems to me that in this situation, one misunderstanding led to another. I have no hard feelings. We all seem to have one goal, to make something better. You are welcome, especially with such a quick response, I appreciate your hard work and thank you for this.