Skip to content

Fix Select widget border not rendering due to BLANK name collision#6359

Closed
charles-001 wants to merge 1 commit into
Textualize:mainfrom
charles-001:fix/select-blank-collision
Closed

Fix Select widget border not rendering due to BLANK name collision#6359
charles-001 wants to merge 1 commit into
Textualize:mainfrom
charles-001:fix/select-blank-collision

Conversation

@charles-001

Copy link
Copy Markdown

Summary

  • Widget.BLANK (a ClassVar[bool] defaulting to False) was added in v7.1.0 to allow widgets to opt into blank rendering
  • Select.BLANK is a pre-existing NoSelection sentinel object used to represent "no value selected"
  • In v7.2.0, render_line and render_lines were optimized to check if self.BLANK: and skip normal rendering — but NoSelection() is truthy, so Select inadvertently enters the blank rendering path, causing its border (and all visual chrome) to disappear
  • Fix: use if self.BLANK is True: to match only the boolean True, not arbitrary truthy objects

Reproduction

from textual.app import App, ComposeResult
from textual.widgets import Select

class TestApp(App):
    CSS = """
    Select { border: heavy #252e49; }
    SelectCurrent { border: none; }
    """
    def compose(self) -> ComposeResult:
        yield Select([("Option 1", 1)], prompt="Test")

TestApp().run()

Expected: Select has a heavy border
Actual (7.2.0+): Select renders with no border

Test plan

  • Verified border renders correctly on 6.11.0 through 7.1.0
  • Verified border disappears on 7.2.0 through 7.5.0
  • Verified fix restores border rendering on 7.5.0
  • Verified Widget.BLANK = True still produces blank rendering for widgets that use it

`Widget.BLANK` (added in 7.1.0, checked in `render_line`/`render_lines`
since 7.2.0) collides with `Select.BLANK`, which is a `NoSelection`
sentinel object. Because `NoSelection` is truthy, the `if self.BLANK:`
check inadvertently matches, causing `Select` to skip normal rendering
and return blank strips — losing its border.

Use `is True` instead of truthiness to ensure only the boolean `True`
triggers blank rendering.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@TomJGooding

Copy link
Copy Markdown
Contributor

Since Widget.BLANK is a ClassVar and should not be overwritten in instances, wouldn't a better solution be to avoid this collision altogether?

@willmcgugan

Copy link
Copy Markdown
Member

Yeah, this doesn't seem the right solution. The right solutions is generally the one you don't have to justify to those that come after you. Having BLANK mean two different things with two different types would be hard to justify.

My fault entirely of course. I wonder why I didn't notice that.

I don't think there is any other choice but to rename Select.BLANK to something else. Suggest Select.BLANK_SELECTION.

This would require a major version bump, which is unfortunate.

@charles-001

Copy link
Copy Markdown
Author

Yeah, I think that makes the most sense Will.

@willmcgugan

Copy link
Copy Markdown
Member

Thanks. Gone with #6374

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.

3 participants