Skip to content

remove RadioButton glyph edges to prevent rendering artifacts in term… #5722

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

Closed
wants to merge 2 commits into from

Conversation

presstab
Copy link

@presstab presstab commented Apr 8, 2025

closes #5721

🛠️ Summary

This PR fixes a rendering issue in the RadioButton widget where an extra line or visual artifact appears around the toggle glyph (●) in certain terminal environments (e.g. Ubuntu 22.04.5 LTS with GNOME Terminal).
✅ Fix

The issue was caused by inherited BUTTON_LEFT and BUTTON_RIGHT characters from the ToggleButton base class. These characters, while visually subtle in some environments, render poorly or as visible lines in others.

To resolve this, BUTTON_LEFT and BUTTON_RIGHT are explicitly set to empty strings in RadioButton:

BUTTON_LEFT = ""
BUTTON_RIGHT = ""

This ensures only the intended center glyph (●) is rendered, avoiding any extra characters or alignment issues.

Bug screenshot:
Screenshot from 2025-04-08 13-48-03

Fixed screenshot:
Screenshot from 2025-04-08 13-11-50

💻 Environment

Tested on:

Ubuntu 22.04.5 LTS

GNOME Terminal (default)

Textual version: [insert version if needed]

Please review the following checklist.

  • Docstrings on all new or modified functions / classes
  • Updated documentation
  • Updated CHANGELOG.md (where appropriate)

@TomJGooding
Copy link
Contributor

TomJGooding commented Apr 8, 2025

These characters aren't "unused", as you can clearly see in the RadioSet example: https://textual.textualize.io/widgets/radioset/#example

But I guess you wouldn't know that, as it looks like this was LLM-generated based on the placeholder for the Textual version you forgot to change.

You're proposing breaking the style of the radio button based on the circle glyph not rendering correctly only in a specific terminal emulator and only with a specific font.

@presstab
Copy link
Author

presstab commented Apr 8, 2025

These characters aren't "unused", as you can clearly see in the RadioSet example: https://textual.textualize.io/widgets/radioset/#example

But I guess you wouldn't know that, as it looks like this was LLM-generated based on the placeholder for the Textual version you forgot to change.

You're proposing breaking the style of the radio button based on the circle glyph not rendering correctly only in a specific terminal emulator and only with a specific font.

Not sure what the hostility is for, I just saw an issue, and thought I could help. I used chatgpt to generate the pr summary, sometimes I don't have the best wording. I also noticed that other PR's here from large contributors use LLM generated PR summaries. I was excited to get this PR together and posted, and forgot to change that tag.

I tracked down this manually (not that it matters), as chatgpt kept trying to change the CSS I didn't feed it Textual source code, so it had limited guesses. I don't see the side buttons used in that example you linked, perhaps you are suggesting they are spacers, I don't know... I am just a first time contributor and I obviously don't have any expertise with the codebase (I am not sure what breaks what, etc).

I would disagree that the default terminal on Ubuntu is some specific use that shouldn't be considered, but don't really care enough to have an argument. I'll just override it in my own project and move on from this topic :)

Edit: just got out my windows machine, and do see the left and right side button fragments rendered. To my untrained eye, that looked like background color rather than characters, so it was not super clear what you were referring to with the radioset link. I see that now. Cheers.

@presstab presstab closed this Apr 8, 2025
@willmcgugan
Copy link
Collaborator

@TomJGooding That was rather ungenerous. I think @presstab 's intentions entirely honorable.

@presstab The issue seems to be that Gnome terminal is rendering that glyph too wide. Having one cell overlap with a subsequent cell just shouldn't happen. So I think the fault lies with GT.

But I don't like to pass the buck with such things. Even if Gnome fix it, it will be a long time before every user gets the update. So finding a workaround is desirable.

Unfortunately we can't make that change, as it would break the output on other terminals. Here are some alternative options as I see it.

  • Find an alternative glyph that happens to work across all terminals, and looks good.
  • Sniff the terminal and change only affected terminals. I generally don't like to do that, as it can be brittle. But it is an option.

I'm going to open the original issue, as it is still valid. We can move the discussion there.

@TomJGooding
Copy link
Contributor

@presstab I'm sorry for jumping to conclusions. Reading this back in the cold light of day, my review was definitely more hostile than I intended. I would hate to think that I'm putting off first time contributors, especially as I'm only a very minor contributor myself.

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.

RadioButton unicode circle display has a line through it
3 participants