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

[css][scrollbar gutter] Manage the scrollbar-gutter: stable CSS property #501

Merged
merged 1 commit into from
May 17, 2022

Conversation

olivierphi
Copy link

@olivierphi olivierphi commented May 11, 2022

(only for vertical content though; we may see later on if we want to also apply that logic for horizontal scrolls?)
https://developer.mozilla.org/en-US/docs/Web/CSS/scrollbar-gutter

closes #421

Demo time!

A text box with and without scrollbar, by default: we can see the text's layout is changing at the end of the first line depending on whether or not the scrollbar must be displayed:

sandbox-basic-scrollbar-gutter.mp4

Now the same text box, with scrollbar-gutter: stable : we can see that the text's layout is stable, with or without scrollbar:

sandbox-basic-scrollbar-gutter-stable.mp4

@olivierphi olivierphi force-pushed the css-add-scrollbar-gutter-property branch 2 times, most recently from 4b7b85f to 6b5cf82 Compare May 12, 2022 09:05
lorem = Text.from_markup(
"""Lorem ipsum dolor sit amet, consectetur adipiscing elit. In velit libero, volutpat nec hendrerit at, faucibus in odio. Aliquam hendrerit nibh sed quam volutpat maximus. Nullam suscipit convallis lorem quis sodales. In tristique lobortis ante et dictum. Ut at finibus ipsum. In urna dolor, placerat et mi facilisis, congue sollicitudin massa. Phasellus felis turpis, cursus eu lectus et, porttitor malesuada augue. Sed feugiat volutpat velit, sollicitudin fringilla velit bibendum faucibus. Lorem ipsum dolor sit amet, consectetur adipiscing elit. In velit libero, volutpat nec hendrerit at, faucibus in odio. Aliquam hendrerit nibh sed quam volutpat maximus. Nullam suscipit convallis lorem quis sodales. In tristique lobortis ante et dictum. Ut at finibus ipsum. In urna dolor, placerat et mi facilisis, congue sollicitudin massa. Phasellus felis turpis, cursus eu lectus et, porttitor malesuada augue. Sed feugiat volutpat velit, sollicitudin fringilla velit bibendum faucibus. """
"""Lorem ipsum dolor sit amet, consectetur adipiscing elit. In velit libero, volutpat nec hendrerit at, faucibus in odio. Aliquam hendrerit nibh sed quam volutpat maximus. Nullam suscipit convallis lorem quis sodales. In tristique lobortis ante et dictum. Ut at finibus ipsum. In urna dolor, placerat et mi facilisis, congue sollicitudin massa. Phasellus felis turpis, cursus eu lectus et, porttitor malesuada augue. Sed feugiat volutpat velit, sollicitudin fringilla velit bibendum faucibus. Lorem ipsum dolor sit amet, consectetur adipiscing elit. In velit libero, volutpat nec hendrerit at, faucibus in odio. Aliquam hendrerit nibh sed quam volutpat maximus. Nullam suscipit convallis lorem quis sodales. In tristique lobortis ante et dictum. Ut at finibus ipsum. In urna dolor, placerat et mi facilisis, congue sollicitudin massa. Phasellus felis turpis, cursus eu lectus et, porttitor malesuada augue. Sed feugiat volutpat velit, sollicitudin fringilla velit bibendum faucibus. """
lorem_short = """Lorem ipsum dolor sit amet, consectetur adipiscing elit. In velit liber a a a, volutpat nec hendrerit at, faucibus in odio. Aliquam hendrerit nibh sed quam volutpat maximus. Nullam suscipit convallis lorem quis sodales. In tristique lobortis ante et dictum. Ut at finibus ipsum."""
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a tiny update to our dear "basic" sandbox: when we press "t" the top text is changed between a "short lorem" and a "long lorem", which allows us to test the layout engine with dynamically updated content - it works pretty well, out of the box! 👌

@@ -135,9 +142,18 @@ async def on_key(self, event) -> None:
def key_d(self):
self.dark = not self.dark

async def key_q(self):
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just because I naturally always press "q" to quit 😅

@@ -770,6 +771,18 @@ def process_align_vertical(self, name: str, tokens: list[Token]) -> None:
process_content_align_horizontal = process_align_horizontal
process_content_align_vertical = process_align_vertical

def process_scrollbar_gutter(self, name: str, tokens: list[Token]) -> None:
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it was interesting to see all the different steps of adding a new CSS property in Textual 🙂

@@ -316,7 +316,7 @@ def replace_rules(
styles = node.styles
base_styles = styles.base

# Styles currently used an new rules
# Styles currently used on new rules
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

quick typo fix as I was reading code here and there to try to understand how to add a new CSS property :-)

@@ -493,6 +493,9 @@ def _arrange_container(self, region: Region) -> Region:
Region: The widget region minus scrollbars.
"""
show_vertical_scrollbar, show_horizontal_scrollbar = self.scrollbars_enabled
if self.styles.scrollbar_gutter == "stable":
# Let's _always_ reserve some space, whether the scrollbar is actually displayed or not:
show_vertical_scrollbar = True
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...and that's the implementation itself! ✨

"overflow_y,scrollbar_gutter,text_length,expected_text_widget_width,expects_vertical_scrollbar",
(
# ------------------------------------------------
# ----- Let's start with `overflow-y: auto`:
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hopefully I've described these test cases well enough to make this test maintainable :-) 🤞

@@ -38,6 +38,9 @@ def __init__(
log_color_system="256",
)

# Let's disable all features by default
self.features = frozenset()
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's make sure that by default our AppTest class has all features disabled - which should make integration tests faster in local environments, as they won't try to connect to the DevTools console even if TEXTUAL=devtools is enabled

@olivierphi olivierphi marked this pull request as ready for review May 12, 2022 09:17
…perty

(only for vertical content though; we may see later on if we want to also apply that logic for horizontal scrolls?)
https://developer.mozilla.org/en-US/docs/Web/CSS/scrollbar-gutter
@olivierphi olivierphi force-pushed the css-add-scrollbar-gutter-property branch from 6b5cf82 to 685a2fb Compare May 12, 2022 09:19
Copy link
Collaborator

@willmcgugan willmcgugan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!

@willmcgugan willmcgugan merged commit c2c2e79 into css May 17, 2022
@willmcgugan willmcgugan deleted the css-add-scrollbar-gutter-property branch May 17, 2022 10:29
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

Successfully merging this pull request may close these issues.

None yet

2 participants