Repository: Textualize/textual
Pull request: User CSS should always take precedence over Widget CSS
Adds an additional (greater) level of specificity (`Specificity5`), which means user-defined CSS (i.e. that inside a `.css` file) _always_ takes precedence over CSS defined inside the `CSS` class var.

Problem statement:
Default versus User CSS
We have CSS specified within a widget (default CSS) and CSS specified in an external file. They are currently considered the same as far as the Textual CSS parser is concerned.

We should probably treat default CSS as always having lower specificity than user specificity. This should make it easier to apply changes to builtin widgets, and is analogous to the browsers default CSS.

We are also probably processing rules in the wrong order. When rules have equal specificity the last rule in the file should take precedence.

Hints from issue discussion:
@darrenburns assigned this to you since it impacts your work on buttons.

Commit message: Add Specificity5 for user defined CSS

Patch to review:
diff --git a/sandbox/darren/buttons.css b/sandbox/darren/buttons.css
new file mode 100644
index 0000000000..cedf20ded1
--- /dev/null
+++ b/sandbox/darren/buttons.css
@@ -0,0 +1,4 @@
+Button {
+    padding-left: 1;
+    padding-right: 1;
+}
diff --git a/sandbox/darren/buttons.py b/sandbox/darren/buttons.py
new file mode 100644
index 0000000000..daf1047a1a
--- /dev/null
+++ b/sandbox/darren/buttons.py
@@ -0,0 +1,31 @@
+from textual import layout, events
+from textual.app import App, ComposeResult
+from textual.widgets import Button
+
+
+class ButtonsApp(App[str]):
+    def compose(self) -> ComposeResult:
+        yield layout.Vertical(
+            Button("default", id="foo"),
+            Button.success("success", id="bar"),
+            Button.warning("warning", id="baz"),
+            Button.error("error", id="baz"),
+        )
+
+    def handle_pressed(self, event: Button.Pressed) -> None:
+        self.app.bell()
+
+    async def on_key(self, event: events.Key) -> None:
+        await self.dispatch_key(event)
+
+    def key_d(self):
+        self.dark = not self.dark
+
+
+app = ButtonsApp(
+    log_path="textual.log", css_path="buttons.css", watch_css=True, log_verbosity=2
+)
+
+if __name__ == "__main__":
+    result = app.run()
+    print(repr(result))
diff --git a/sandbox/file_search.py b/sandbox/darren/file_search.py
similarity index 100%
rename from sandbox/file_search.py
rename to sandbox/darren/file_search.py
diff --git a/sandbox/file_search.scss b/sandbox/darren/file_search.scss
similarity index 100%
rename from sandbox/file_search.scss
rename to sandbox/darren/file_search.scss
diff --git a/sandbox/focus_keybindings.py b/sandbox/darren/focus_keybindings.py
similarity index 100%
rename from sandbox/focus_keybindings.py
rename to sandbox/darren/focus_keybindings.py
diff --git a/sandbox/focus_keybindings.scss b/sandbox/darren/focus_keybindings.scss
similarity index 100%
rename from sandbox/focus_keybindings.scss
rename to sandbox/darren/focus_keybindings.scss
diff --git a/sandbox/darren/just_a_box.css b/sandbox/darren/just_a_box.css
new file mode 100644
index 0000000000..765fd56517
--- /dev/null
+++ b/sandbox/darren/just_a_box.css
@@ -0,0 +1,6 @@
+#box {
+    height: 50%;
+    width: 50%;
+    align: center middle;
+    background: green;
+}
diff --git a/sandbox/darren/just_a_box.py b/sandbox/darren/just_a_box.py
new file mode 100644
index 0000000000..0750afc43e
--- /dev/null
+++ b/sandbox/darren/just_a_box.py
@@ -0,0 +1,29 @@
+from __future__ import annotations
+
+from rich.console import RenderableType
+from rich.panel import Panel
+
+from textual.app import App, ComposeResult
+from textual.widget import Widget
+
+
+class Box(Widget):
+    CSS = "#box {background: blue;}"
+
+    def __init__(
+        self, id: str | None = None, classes: str | None = None, *children: Widget
+    ):
+        super().__init__(*children, id=id, classes=classes)
+
+    def render(self) -> RenderableType:
+        return Panel("Box")
+
+
+class JustABox(App):
+    def compose(self) -> ComposeResult:
+        yield Box(id="box")
+
+
+if __name__ == "__main__":
+    app = JustABox(css_path="just_a_box.css", watch_css=True)
+    app.run()
diff --git a/sandbox/tabs.py b/sandbox/darren/tabs.py
similarity index 100%
rename from sandbox/tabs.py
rename to sandbox/darren/tabs.py
diff --git a/sandbox/tabs.scss b/sandbox/darren/tabs.scss
similarity index 100%
rename from sandbox/tabs.scss
rename to sandbox/darren/tabs.scss
diff --git a/sandbox/hsl.py b/sandbox/hsl.py
deleted file mode 100644
index 17c0d4f495..0000000000
--- a/sandbox/hsl.py
+++ /dev/null
@@ -1,11 +0,0 @@
-from textual.app import App, ComposeResult
-from textual.widgets import Static
-
-
-class HSLApp(App):
-    def compose(self) -> ComposeResult:
-        yield Static(classes="box")
-
-
-app = HSLApp(css_path="hsl.scss", watch_css=True)
-app.run()
diff --git a/sandbox/hsl.scss b/sandbox/hsl.scss
deleted file mode 100644
index 78c0a495f3..0000000000
--- a/sandbox/hsl.scss
+++ /dev/null
@@ -1,5 +0,0 @@
-.box {
-  height: 1fr;
-  /*background: rgb(180,50, 50);*/
-  background: hsl(180,50%, 50%);
-}
diff --git a/src/textual/css/model.py b/src/textual/css/model.py
index 89623ac95d..bf4e9e7c83 100644
--- a/src/textual/css/model.py
+++ b/src/textual/css/model.py
@@ -156,6 +156,7 @@ class RuleSet:
     styles: Styles = field(default_factory=Styles)
     errors: list[tuple[Token, str]] = field(default_factory=list)
     classes: set[str] = field(default_factory=set)
+    is_widget_rule_set: bool = False
 
     @classmethod
     def _selector_to_css(cls, selectors: list[Selector]) -> str:
diff --git a/src/textual/css/parse.py b/src/textual/css/parse.py
index f3ce8d3f70..9052c3b1dd 100644
--- a/src/textual/css/parse.py
+++ b/src/textual/css/parse.py
@@ -7,6 +7,7 @@
 from rich import print
 
 from textual.css.errors import UnresolvedVariableError
+from textual.css.types import Specificity3
 from ._styles_builder import StylesBuilder, DeclarationError
 from .model import (
     Declaration,
@@ -20,7 +21,7 @@
 from .tokenize import tokenize, tokenize_declarations, Token, tokenize_values
 from .tokenizer import EOFError, ReferencedBy
 
-SELECTOR_MAP: dict[str, tuple[SelectorType, tuple[int, int, int]]] = {
+SELECTOR_MAP: dict[str, tuple[SelectorType, Specificity3]] = {
     "selector": (SelectorType.TYPE, (0, 0, 1)),
     "selector_start": (SelectorType.TYPE, (0, 0, 1)),
     "selector_class": (SelectorType.CLASS, (0, 1, 0)),
diff --git a/src/textual/css/styles.py b/src/textual/css/styles.py
index 30a2f22347..4530bacd2a 100644
--- a/src/textual/css/styles.py
+++ b/src/textual/css/styles.py
@@ -56,6 +56,7 @@
     AlignVertical,
     Visibility,
     ScrollbarGutter,
+    Specificity5,
 )
 
 if sys.version_info >= (3, 8):
@@ -511,20 +512,27 @@ def merge_rules(self, rules: RulesMap) -> None:
         self._rules.update(rules)
 
     def extract_rules(
-        self, specificity: Specificity3
-    ) -> list[tuple[str, Specificity4, Any]]:
-        """Extract rules from Styles object, and apply !important css specificity.
+        self,
+        specificity: Specificity3,
+        is_widget_rule: bool = False,
+    ) -> list[tuple[str, Specificity5, Any]]:
+        """Extract rules from Styles object, and apply !important css specificity as
+        well as higher specificity of user CSS vs widget CSS.
 
         Args:
             specificity (Specificity3): A node specificity.
 
         Returns:
-            list[tuple[str, Specificity4, Any]]]: A list containing a tuple of <RULE NAME>, <SPECIFICITY> <RULE VALUE>.
+            list[tuple[str, Specificity5, Any]]]: A list containing a tuple of <RULE NAME>, <SPECIFICITY> <RULE VALUE>.
         """
         is_important = self.important.__contains__
 
         rules = [
-            (rule_name, (int(is_important(rule_name)), *specificity), rule_value)
+            (
+                rule_name,
+                (int(not is_widget_rule), int(is_important(rule_name)), *specificity),
+                rule_value,
+            )
             for rule_name, rule_value in self._rules.items()
         ]
 
diff --git a/src/textual/css/stylesheet.py b/src/textual/css/stylesheet.py
index b627a6e8ff..bc2f6e61c2 100644
--- a/src/textual/css/stylesheet.py
+++ b/src/textual/css/stylesheet.py
@@ -122,6 +122,8 @@ def __init__(self, *, variables: dict[str, str] | None = None) -> None:
         self._rules: list[RuleSet] = []
         self.variables = variables or {}
         self.source: dict[str, str] = {}
+        # Records which of the source keys represent CSS defined at the widget level
+        self._widget_css_paths: set[str] = set()
         self._require_parse = False
 
     def __rich_repr__(self) -> rich.repr.Result:
@@ -147,6 +149,7 @@ def copy(self) -> Stylesheet:
         """
         stylesheet = Stylesheet(variables=self.variables.copy())
         stylesheet.source = self.source.copy()
+        stylesheet._widget_css_paths = self._widget_css_paths.copy()
         return stylesheet
 
     def set_variables(self, variables: dict[str, str]) -> None:
@@ -160,7 +163,6 @@ def set_variables(self, variables: dict[str, str]) -> None:
     def _parse_rules(self, css: str, path: str | PurePath) -> list[RuleSet]:
         """Parse CSS and return rules.
 
-
         Args:
             css (str): String containing Textual CSS.
             path (str | PurePath): Path to CSS or unique identifier
@@ -177,6 +179,11 @@ def _parse_rules(self, css: str, path: str | PurePath) -> list[RuleSet]:
             raise
         except Exception as error:
             raise StylesheetError(f"failed to parse css; {error}")
+
+        if path in self._widget_css_paths:
+            for rule in rules:
+                rule.is_widget_rule_set = True
+
         return rules
 
     def read(self, filename: str | PurePath) -> None:
@@ -199,13 +206,17 @@ def read(self, filename: str | PurePath) -> None:
         self.source[str(path)] = css
         self._require_parse = True
 
-    def add_source(self, css: str, path: str | PurePath | None = None) -> None:
+    def add_source(
+        self, css: str, path: str | PurePath | None = None, is_widget_css: bool = False
+    ) -> None:
         """Parse CSS from a string.
 
         Args:
             css (str): String with CSS source.
             path (str | PurePath, optional): The path of the source if a file, or some other identifier.
                 Defaults to None.
+            is_widget_css (bool): True if the CSS is defined in the Widget, False if the CSS is defined
+                in a user stylesheet.
 
         Raises:
             StylesheetError: If the CSS could not be read.
@@ -220,6 +231,11 @@ def add_source(self, css: str, path: str | PurePath | None = None) -> None:
             # Path already in source, and CSS is identical
             return
 
+        # Record any CSS defined at the widget level for
+        # different specificity treatment from user CSS.
+        if is_widget_css:
+            self._widget_css_paths.add(path)
+
         self.source[path] = css
         self._require_parse = True
 
@@ -286,9 +302,10 @@ def apply(self, node: DOMNode, animate: bool = False) -> None:
 
         # Collect the rules defined in the stylesheet
         for rule in reversed(self.rules):
-            for specificity in _check_rule(rule, node):
+            is_widget_rule = rule.is_widget_rule_set
+            for base_specificity in _check_rule(rule, node):
                 for key, rule_specificity, value in rule.styles.extract_rules(
-                    specificity
+                    base_specificity, is_widget_rule
                 ):
                     rule_attributes[key].append((rule_specificity, value))
 
diff --git a/src/textual/css/types.py b/src/textual/css/types.py
index e7fd749b2d..1463bf6fa5 100644
--- a/src/textual/css/types.py
+++ b/src/textual/css/types.py
@@ -37,5 +37,7 @@
 BoxSizing = Literal["border-box", "content-box"]
 Overflow = Literal["scroll", "hidden", "auto"]
 EdgeStyle = Tuple[EdgeType, Color]
+
 Specificity3 = Tuple[int, int, int]
 Specificity4 = Tuple[int, int, int, int]
+Specificity5 = Tuple[int, int, int, int, int]
diff --git a/src/textual/widget.py b/src/textual/widget.py
index 2100f70fd4..11d79780c8 100644
--- a/src/textual/widget.py
+++ b/src/textual/widget.py
@@ -74,12 +74,12 @@ class Widget(DOMNode):
     CSS = """
     Widget{
         scrollbar-background: $panel-darken-2;
-        scrollbar-background-hover: $panel-darken-3;        
+        scrollbar-background-hover: $panel-darken-3;
         scrollbar-color: $system;
-        scrollbar-color-active: $secondary-darken-1;                
+        scrollbar-color-active: $secondary-darken-1;
         scrollbar-size-vertical: 2;
         scrollbar-size-horizontal: 1;
-        
+
     }
     """
 
@@ -194,7 +194,7 @@ def on_register(self, app: App) -> None:
         """
         # Parse the Widget's CSS
         for path, css in self.css:
-            self.app.stylesheet.add_source(css, path=path)
+            self.app.stylesheet.add_source(css, path=path, is_widget_css=True)
 
     def get_box_model(
         self, container: Size, viewport: Size, fraction_unit: Fraction
diff --git a/tests/css/test_stylesheet.py b/tests/css/test_stylesheet.py
index 6d4425b426..fd98e89041 100644
--- a/tests/css/test_stylesheet.py
+++ b/tests/css/test_stylesheet.py
@@ -3,12 +3,15 @@
 
 import pytest
 
+from tests.utilities.test_app import AppTest
+from textual.app import App, ComposeResult
 from textual.color import Color
 from textual.css._help_renderables import HelpText
 from textual.css.stylesheet import Stylesheet, StylesheetParseError
 from textual.css.tokenizer import TokenizeError
 from textual.dom import DOMNode
 from textual.geometry import Spacing
+from textual.widget import Widget
 
 
 def _make_stylesheet(css: str) -> Stylesheet:
@@ -101,6 +104,27 @@ def test_stylesheet_apply_empty_rulesets():
     stylesheet.apply(node)
 
 
+def test_stylesheet_apply_user_css_over_widget_css():
+    user_css = ".a {color: red; tint: yellow;}"
+
+    class MyWidget(Widget):
+        CSS = ".a {color: blue; background: lime;}"
+
+    node = MyWidget()
+    node.add_class("a")
+
+    stylesheet = _make_stylesheet(user_css)
+    stylesheet.add_source(MyWidget.CSS, "widget.py:MyWidget", is_widget_css=True)
+    stylesheet.apply(node)
+
+    # The node is red because user CSS overrides Widget.CSS
+    assert node.styles.color == Color(255, 0, 0)
+    # The background colour defined in the Widget still applies, since user CSS doesn't override it
+    assert node.styles.background == Color(0, 255, 0)
+    # As expected, the tint colour is yellow, since there's no competition between user or widget CSS
+    assert node.styles.tint == Color(255, 255, 0)
+
+
 @pytest.mark.parametrize(
     "css_value,expectation,expected_color",
     [


Code review comments:
diff_hunk: @@ -199,13 +206,17 @@ def read(self, filename: str | PurePath) -> None:
         self.source[str(path)] = css
         self._require_parse = True
 
-    def add_source(self, css: str, path: str | PurePath | None = None) -> None:
+    def add_source(
+        self, css: str, path: str | PurePath | None = None, is_widget_css: bool = False
path: src/textual/css/stylesheet.py
text: @user1:
An `App` can have CSS, which isn't technically a widget.

Suggest a name which doesn't refer to widgets / nodes. Something like `is_defaults`

@author:
Good point

@author:
Not sure about is_defaults though, sounds easy to conflate with the CSS default values from the style property descriptors? Wonder if we could have a different name for "the browsers CSS".

@user1:
"is_default_rules" maybe? We can always change it later if we come up with a better name.

@user1:
"is_default_rules" maybe? We can always change it later if we come up with a better name.

diff_hunk: @@ -177,6 +179,11 @@ def _parse_rules(self, css: str, path: str | PurePath) -> list[RuleSet]:
             raise
         except Exception as error:
             raise StylesheetError(f"failed to parse css; {error}")
+
+        if path in self._widget_css_paths:
path: src/textual/css/stylesheet.py
text: @user1:
What if `_parse_rules` could take a boolean indicating that it was Widget CSS, and pass that in to `parse` which would store it in the rule? I think that would negate this check and loop.

diff_hunk: @@ -511,20 +512,27 @@ def merge_rules(self, rules: RulesMap) -> None:
         self._rules.update(rules)
 
     def extract_rules(
-        self, specificity: Specificity3
-    ) -> list[tuple[str, Specificity4, Any]]:
-        """Extract rules from Styles object, and apply !important css specificity.
+        self,
+        specificity: Specificity3,
+        is_widget_rule: bool = False,
+    ) -> list[tuple[str, Specificity5, Any]]:
+        """Extract rules from Styles object, and apply !important css specificity as
+        well as higher specificity of user CSS vs widget CSS.
 
         Args:
             specificity (Specificity3): A node specificity.
 
         Returns:
-            list[tuple[str, Specificity4, Any]]]: A list containing a tuple of <RULE NAME>, <SPECIFICITY> <RULE VALUE>.
+            list[tuple[str, Specificity5, Any]]]: A list containing a tuple of <RULE NAME>, <SPECIFICITY> <RULE VALUE>.
         """
         is_important = self.important.__contains__
 
         rules = [
-            (rule_name, (int(is_important(rule_name)), *specificity), rule_value)
+            (
+                rule_name,
+                (int(not is_widget_rule), int(is_important(rule_name)), *specificity),
path: src/textual/css/styles.py
text: @user1:
Just discovered something interesting. `0 if widget_rule else 1` is dramatically faster than `int(not is_widget_rule)` 

I suspect because it avoids the global lookup of `int`

Would you mind both calls to int? 

When you look after the nano seconds, the microseconds look after themselves!

@author:
Done :)

diff_hunk: @@ -122,6 +122,8 @@ def __init__(self, *, variables: dict[str, str] | None = None) -> None:
         self._rules: list[RuleSet] = []
         self.variables = variables or {}
         self.source: dict[str, str] = {}
+        # Records which of the source keys represent CSS defined at the widget level
+        self._widget_css_paths: set[str] = set()
path: src/textual/css/stylesheet.py
text: @user1:
Rather than a new collection, could you store this in `self.source`? i.e. make it a `dict[str, tuple[str, int]]`

@author:
I was actually on the fence about doing something like this but opted against it as it made the lookups into `self.source` a bit uglier and there were a good few of them. It's a trade-off though - happy to make the change though if you prefer it.

@user1:
Let’s give it a shot 👍

@author:
No objections to a NamedTuple instead?

@user1:
I try not to use too many namedtuples and dataclasses, because they do add a slight penalty to startup. But if you find it makes the code more readable, go for it...