From 5a355da78bbe9d6e1bcf5b70f22549a2a8337b55 Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Mon, 1 May 2023 14:17:29 +0100 Subject: [PATCH 01/12] Fixed `!important` not applying to `border` --- CHANGELOG.md | 5 +++++ src/textual/css/_styles_builder.py | 10 ++++++++++ 2 files changed, 15 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a39253ca0..f5de8f883 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,11 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +## Unreleased + +### Fixed + +- Fixed `!important` not applying to `border` https://github.com/Textualize/textual/issues/2420 ## [0.22.3] - 2023-04-29 diff --git a/src/textual/css/_styles_builder.py b/src/textual/css/_styles_builder.py index 4e6ddcc1d..3bb185432 100644 --- a/src/textual/css/_styles_builder.py +++ b/src/textual/css/_styles_builder.py @@ -486,6 +486,16 @@ class StylesBuilder: rules = self.styles._rules rules["border_top"] = rules["border_right"] = border rules["border_bottom"] = rules["border_left"] = border + # If border is marked as important... + if "border" in self.styles.important: + # ...border alone at this depth isn't really a thing, only + # border edges (that's to say, border gets expanded out to all 4 + # edges). So we remove "border" as important and mark all the + # edges as important. + self.styles.important.remove("border") + self.styles.important.update( + f"border_{edge}" for edge in ("top", "left", "bottom", "right") + ) def process_border_top(self, name: str, tokens: list[Token]) -> None: self._process_border_edge("top", name, tokens) From 883e293e1fd720ee9aa0a0bd4f922af2a3c17738 Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Mon, 1 May 2023 15:00:07 +0100 Subject: [PATCH 02/12] Move the important distribution code into its own method Border isn't the only place we're going to want to do this, so let's now turn this into a more general body of code and have border use it. --- src/textual/css/_styles_builder.py | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/src/textual/css/_styles_builder.py b/src/textual/css/_styles_builder.py index 3bb185432..fc2100288 100644 --- a/src/textual/css/_styles_builder.py +++ b/src/textual/css/_styles_builder.py @@ -481,21 +481,29 @@ class StylesBuilder: border = self._parse_border(name, tokens) self.styles._rules[f"border_{edge}"] = border # type: ignore + def _distribute_edge_importance(self, style_name: str) -> None: + """Distribute importance amongst all edges of a rule. + + Args: + style_name: The style name that has edges. + + Styles such as border and outline don't really result in `border` or + `outline` as rules, instead splitting out the style equally to the + edged versions. This method, upon finding the style name in the + important set, removes it and adds the edged versions there instead. + """ + if style_name in self.styles.important: + self.styles.important.remove(style_name) + self.styles.important.update( + f"{style_name}_{edge}" for edge in ("top", "left", "bottom", "right") + ) + def process_border(self, name: str, tokens: list[Token]) -> None: border = self._parse_border(name, tokens) rules = self.styles._rules rules["border_top"] = rules["border_right"] = border rules["border_bottom"] = rules["border_left"] = border - # If border is marked as important... - if "border" in self.styles.important: - # ...border alone at this depth isn't really a thing, only - # border edges (that's to say, border gets expanded out to all 4 - # edges). So we remove "border" as important and mark all the - # edges as important. - self.styles.important.remove("border") - self.styles.important.update( - f"border_{edge}" for edge in ("top", "left", "bottom", "right") - ) + self._distribute_edge_importance("border") def process_border_top(self, name: str, tokens: list[Token]) -> None: self._process_border_edge("top", name, tokens) From 7a69c037b824f2e556dad62f13dcff65d5f30095 Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Mon, 1 May 2023 15:00:54 +0100 Subject: [PATCH 03/12] Distribute importance amongst all sides of an outline --- src/textual/css/_styles_builder.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/textual/css/_styles_builder.py b/src/textual/css/_styles_builder.py index fc2100288..2497c26da 100644 --- a/src/textual/css/_styles_builder.py +++ b/src/textual/css/_styles_builder.py @@ -526,6 +526,7 @@ class StylesBuilder: rules = self.styles._rules rules["outline_top"] = rules["outline_right"] = border rules["outline_bottom"] = rules["outline_left"] = border + self._distribute_edge_importance("outline") def process_outline_top(self, name: str, tokens: list[Token]) -> None: self._process_outline("top", name, tokens) From d6f304f5d1120787f9c924c5c70a69cbde02c396 Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Mon, 1 May 2023 15:19:24 +0100 Subject: [PATCH 04/12] Update the CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f5de8f883..c0fd186d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Fixed - Fixed `!important` not applying to `border` https://github.com/Textualize/textual/issues/2420 +- Fixed `!important` not applying to `outline` https://github.com/Textualize/textual/issues/2420 ## [0.22.3] - 2023-04-29 From d6eb44bd1968c96d2af85f0b208c3f62f2195376 Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Mon, 1 May 2023 15:21:12 +0100 Subject: [PATCH 05/12] Fix outline-right not being recognised See #2446. --- CHANGELOG.md | 1 + src/textual/css/_styles_builder.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c0fd186d5..487a40ff8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Fixed `!important` not applying to `border` https://github.com/Textualize/textual/issues/2420 - Fixed `!important` not applying to `outline` https://github.com/Textualize/textual/issues/2420 +- Fixed `outline-right` not being recognised https://github.com/Textualize/textual/issues/2446 ## [0.22.3] - 2023-04-29 diff --git a/src/textual/css/_styles_builder.py b/src/textual/css/_styles_builder.py index 2497c26da..2b987d799 100644 --- a/src/textual/css/_styles_builder.py +++ b/src/textual/css/_styles_builder.py @@ -531,7 +531,7 @@ class StylesBuilder: def process_outline_top(self, name: str, tokens: list[Token]) -> None: self._process_outline("top", name, tokens) - def process_parse_border_right(self, name: str, tokens: list[Token]) -> None: + def process_outline_right(self, name: str, tokens: list[Token]) -> None: self._process_outline("right", name, tokens) def process_outline_bottom(self, name: str, tokens: list[Token]) -> None: From 27ca4969a8a0c3d784fe70712c062e5ee358d67f Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Tue, 2 May 2023 09:36:08 +0100 Subject: [PATCH 06/12] Add tests for styles that have "sub-styles" and !important This is a series of tests for checking styles that have sub-styles, or sub-parts, or whatever the correct name would be; the testing being that if !important is applied to the whole, that it works. Starting with #2420 it became apparent that this didn't work as intended, and once that work started it became obvious that it affected more than just border. So these tests test all of the styles that can be specified as a single whole, or as a set of parts (sides, directions, etc). --- tests/test_style_importance.py | 102 +++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 tests/test_style_importance.py diff --git a/tests/test_style_importance.py b/tests/test_style_importance.py new file mode 100644 index 000000000..fc5547416 --- /dev/null +++ b/tests/test_style_importance.py @@ -0,0 +1,102 @@ +from textual.app import App, ComposeResult +from textual.color import Color +from textual.containers import Container +from textual.css.scalar import Scalar, ScalarOffset + + +class StyleApp(App[None]): + CSS = """ + Container { + border: round green !important; + outline: round green !important; + align: right bottom !important; + content-align: right bottom !important; + offset: 17 23 !important; + overflow: hidden hidden !important; + padding: 10 20 30 40 !important; + scrollbar-size: 23 42 !important; + } + + Container.more-specific { + border: solid red; + outline: solid red; + align: center middle; + content-align: center middle; + offset: 0 0; + overflow: scroll scroll; + padding: 1 2 3 4; + scrollbar-size: 1 2; + } + """ + + def compose(self) -> ComposeResult: + yield Container(classes="more-specific") + + +async def test_border_importance(): + """Border without sides should support !important""" + async with StyleApp().run_test() as pilot: + border = pilot.app.query_one(Container).styles.border + desired = ("round", Color.parse("green")) + assert border.top == desired + assert border.left == desired + assert border.bottom == desired + assert border.right == desired + + +async def test_outline_importance(): + """Outline without sides should support !important""" + async with StyleApp().run_test() as pilot: + outline = pilot.app.query_one(Container).styles.outline + desired = ("round", Color.parse("green")) + assert outline.top == desired + assert outline.left == desired + assert outline.bottom == desired + assert outline.right == desired + + +async def test_align_importance(): + """Align without direction should support !important""" + async with StyleApp().run_test() as pilot: + assert pilot.app.query_one(Container).styles.align == ("right", "bottom") + + +async def test_content_align_importance(): + """Content align without direction should support !important""" + async with StyleApp().run_test() as pilot: + assert pilot.app.query_one(Container).styles.content_align == ( + "right", + "bottom", + ) + + +async def test_offset_importance(): + """Offset without direction should support !important""" + async with StyleApp().run_test() as pilot: + assert pilot.app.query_one(Container).styles.offset == ScalarOffset.from_offset( + (17, 23) + ) + + +async def test_overflow_importance(): + """Overflow without direction should support !important""" + async with StyleApp().run_test() as pilot: + assert pilot.app.query_one(Container).styles.overflow_x == "hidden" + assert pilot.app.query_one(Container).styles.overflow_y == "hidden" + + +async def test_padding_importance(): + """Padding without sides should support !important""" + async with StyleApp().run_test() as pilot: + padding = pilot.app.query_one(Container).styles.padding + assert padding.top == 10 + assert padding.left == 40 + assert padding.bottom == 30 + assert padding.right == 20 + + +async def test_scrollbar_size_importance(): + """Scrollbar size without direction should support !important""" + async with StyleApp().run_test() as pilot: + assert pilot.app.query_one(Container).styles.scrollbar_size_horizontal == 23 + assert pilot.app.query_one(Container).styles.scrollbar_size_vertical == 42 From 8fc6e195e17c9da4358617d715e3f2c6fb99eb94 Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Tue, 2 May 2023 09:49:06 +0100 Subject: [PATCH 07/12] Change the importance distribution method to be more generic --- src/textual/css/_styles_builder.py | 38 +++++++++++++++--------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/textual/css/_styles_builder.py b/src/textual/css/_styles_builder.py index 2b987d799..0f03e4419 100644 --- a/src/textual/css/_styles_builder.py +++ b/src/textual/css/_styles_builder.py @@ -252,6 +252,23 @@ class StylesBuilder: else: scalar_error() + def _distribute_importance(self, prefix: str, suffixes: tuple[str, ...]) -> None: + """Distribute importance amongst all aspects of the given style. + + Args: + prefix: The prefix of the style. + siffixes: The suffixes to distribute amongst. + + A number of styles can be set with the 'prefix' of the style, + providing the values as a series of parameters; or they can be set + with specific suffixes. Think `border` vs `border-left`, etc. This + method is used to ensure that if the former is set, `!important` is + distributed amongst all the suffixes. + """ + if prefix in self.styles.important: + self.styles.important.remove(prefix) + self.styles.important.update(f"{prefix}_{suffix}" for suffix in suffixes) + def process_box_sizing(self, name: str, tokens: list[Token]) -> None: for token in tokens: name, value, _, _, location, _ = token @@ -481,29 +498,12 @@ class StylesBuilder: border = self._parse_border(name, tokens) self.styles._rules[f"border_{edge}"] = border # type: ignore - def _distribute_edge_importance(self, style_name: str) -> None: - """Distribute importance amongst all edges of a rule. - - Args: - style_name: The style name that has edges. - - Styles such as border and outline don't really result in `border` or - `outline` as rules, instead splitting out the style equally to the - edged versions. This method, upon finding the style name in the - important set, removes it and adds the edged versions there instead. - """ - if style_name in self.styles.important: - self.styles.important.remove(style_name) - self.styles.important.update( - f"{style_name}_{edge}" for edge in ("top", "left", "bottom", "right") - ) - def process_border(self, name: str, tokens: list[Token]) -> None: border = self._parse_border(name, tokens) rules = self.styles._rules rules["border_top"] = rules["border_right"] = border rules["border_bottom"] = rules["border_left"] = border - self._distribute_edge_importance("border") + self._distribute_importance("border", ("top", "left", "bottom", "right")) def process_border_top(self, name: str, tokens: list[Token]) -> None: self._process_border_edge("top", name, tokens) @@ -526,7 +526,7 @@ class StylesBuilder: rules = self.styles._rules rules["outline_top"] = rules["outline_right"] = border rules["outline_bottom"] = rules["outline_left"] = border - self._distribute_edge_importance("outline") + self._distribute_importance("outline", ("top", "left", "bottom", "right")) def process_outline_top(self, name: str, tokens: list[Token]) -> None: self._process_outline("top", name, tokens) From 763f9b012fe1fb75d45b91972d10c006d95a14c0 Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Tue, 2 May 2023 10:06:17 +0100 Subject: [PATCH 08/12] Distribute !important on align --- src/textual/css/_styles_builder.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/textual/css/_styles_builder.py b/src/textual/css/_styles_builder.py index 0f03e4419..23ff9544f 100644 --- a/src/textual/css/_styles_builder.py +++ b/src/textual/css/_styles_builder.py @@ -811,6 +811,8 @@ class StylesBuilder: self.styles._rules[f"{name}_horizontal"] = token_horizontal.value # type: ignore self.styles._rules[f"{name}_vertical"] = token_vertical.value # type: ignore + self._distribute_importance("align", ("horizontal", "vertical")) + def process_align_horizontal(self, name: str, tokens: list[Token]) -> None: try: value = self._process_enum(name, tokens, VALID_ALIGN_HORIZONTAL) From ad37b6809a2328b26d81988662b0c5c5e68a0c5b Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Tue, 2 May 2023 10:08:33 +0100 Subject: [PATCH 09/12] Distribute !important on content-align --- src/textual/css/_styles_builder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/textual/css/_styles_builder.py b/src/textual/css/_styles_builder.py index 23ff9544f..00685752b 100644 --- a/src/textual/css/_styles_builder.py +++ b/src/textual/css/_styles_builder.py @@ -811,7 +811,7 @@ class StylesBuilder: self.styles._rules[f"{name}_horizontal"] = token_horizontal.value # type: ignore self.styles._rules[f"{name}_vertical"] = token_vertical.value # type: ignore - self._distribute_importance("align", ("horizontal", "vertical")) + self._distribute_importance(name, ("horizontal", "vertical")) def process_align_horizontal(self, name: str, tokens: list[Token]) -> None: try: From cbe418de184fcf568ce5710a05aa30771caa2d81 Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Tue, 2 May 2023 10:12:18 +0100 Subject: [PATCH 10/12] Distribute !important on overflow --- src/textual/css/_styles_builder.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/textual/css/_styles_builder.py b/src/textual/css/_styles_builder.py index 00685752b..bf99900aa 100644 --- a/src/textual/css/_styles_builder.py +++ b/src/textual/css/_styles_builder.py @@ -321,6 +321,7 @@ class StylesBuilder: ) rules["overflow_x"] = cast(Overflow, overflow_x) rules["overflow_y"] = cast(Overflow, overflow_y) + self._distribute_importance("overflow", ("x", "y")) def process_overflow_x(self, name: str, tokens: list[Token]) -> None: self.styles._rules["overflow_x"] = cast( From 16d62dfb203133e1fe3aeecc69b9bb3415ae1f19 Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Tue, 2 May 2023 10:13:24 +0100 Subject: [PATCH 11/12] Distribute !important on scrollbar size --- src/textual/css/_styles_builder.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/textual/css/_styles_builder.py b/src/textual/css/_styles_builder.py index bf99900aa..f38506311 100644 --- a/src/textual/css/_styles_builder.py +++ b/src/textual/css/_styles_builder.py @@ -881,6 +881,7 @@ class StylesBuilder: scrollbar_size_error(name, token2) self.styles._rules["scrollbar_size_horizontal"] = horizontal self.styles._rules["scrollbar_size_vertical"] = vertical + self._distribute_importance("scrollbar_size", ("horizontal", "vertical")) def process_scrollbar_size_vertical(self, name: str, tokens: list[Token]) -> None: if not tokens: From 87847cea4cc363dbb75dc58d1df3c376690de9ab Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Tue, 2 May 2023 10:16:45 +0100 Subject: [PATCH 12/12] Update the CHANGELOG --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f893e33f..0d82e48c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,8 +9,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Fixed +- Fixed `!important` not applying to `align` https://github.com/Textualize/textual/issues/2420 - Fixed `!important` not applying to `border` https://github.com/Textualize/textual/issues/2420 +- Fixed `!important` not applying to `content-align` https://github.com/Textualize/textual/issues/2420 - Fixed `!important` not applying to `outline` https://github.com/Textualize/textual/issues/2420 +- Fixed `!important` not applying to `overflow` https://github.com/Textualize/textual/issues/2420 +- Fixed `!important` not applying to `scrollbar-size` https://github.com/Textualize/textual/issues/2420 - Fixed `outline-right` not being recognised https://github.com/Textualize/textual/issues/2446 ### Changed