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

Rendering issue with soft hyphens (~~long/wrapped text in OptionList~~) #4073

Closed
davep opened this issue Jan 29, 2024 · 13 comments
Closed

Rendering issue with soft hyphens (~~long/wrapped text in OptionList~~) #4073

davep opened this issue Jan 29, 2024 · 13 comments
Assignees
Labels
bug Something isn't working question Further information is requested Task

Comments

@davep
Copy link
Contributor

davep commented Jan 29, 2024

At the suggestion of @willmcgugan I'm recording this without an MRE (although I have tried to make one; see below). In an application I'm currently toying with, over the weekend, I ran into this curious rendering bug:

Screenshot 2024-01-28 at 12 42 56

Note the effect seen to the right of question 9.

The widget is an OptionList, the Options within are made up of Rich renderables which in turn are made up of a Group that contains two Table.grids.

I've attempted to isolate the issue from the wider application, adding all the relevant styles (also doing it as a ModalScreen as this was part of such):

from rich.table import Table
from rich.console import Group

from textual.app import App, ComposeResult
from textual.containers import Vertical
from textual.screen import ModalScreen
from textual.widgets import OptionList

NAMES = [
    "Cardiff",
    "Swansea",
    "Aberdare",
    "Llanfairpwllgwyngyllgogerychwyrndrobwllllantysiliogogogoch",
    "Mountain Ash",
    "Hirwaun"
]

class Names(ModalScreen[None]):

    CSS = """
    Names {
        align: center middle;

        Vertical {
            width: auto;
            max-width: 60%;
            height: auto;
            max-height: 80%;
            padding: 1 2 0 2;
            background: $surface;
            border: panel $primary;
            border-title-color: $accent;

            OptionList {
                height: 1fr;
                margin: 1 0 1 0;
                &> .option-list--option {
                    padding: 0 5 1 0;
                }
            }
        }
    }
    """

    def gridify(self, n: int, name: str) -> Group:
        question = Table.grid()
        question.add_column(width=3, justify="right")
        question.add_column(width=1)
        question.add_column(ratio=1)
        question.add_row(str(n), "", f"{name} is the name of a place in Wales." )
        answer = Table.grid()
        answer.add_column(width=4)
        answer.add_column(ratio=1)
        answer.add_row("", "[red]Here is the[/] [green]second line[/]")
        return Group(question, answer)

    def compose(self) -> ComposeResult:
        with Vertical():
            yield OptionList(*[self.gridify(n, name) for n, name in enumerate(NAMES)])

class TruncatedTextOptionApp(App[None]):

    CSS = """
    Screen#_default {
        background: red;
    }
    """

    def on_mount(self) -> None:
        self.push_screen(Names())

if __name__ == "__main__":
    TruncatedTextOptionApp().run()

the result however seems fine:

Screenshot 2024-01-29 at 10 20 24

@davep davep added bug Something isn't working question Further information is requested Task labels Jan 29, 2024
davep added a commit to davep/textual-sandbox that referenced this issue Jan 29, 2024
@davep
Copy link
Contributor Author

davep commented Jan 29, 2024

Update: I'm starting to suspect it doesn't have much to do with long/wrapped text at all, and it could be there is a problematic character or similar in the underlying data. I'm thinking this based on the fact that, while testing/developing some more just now, I got the same issue, only this time, if I make the window wide enough that the suspect word itself doesn't overflow, I still see the rendering issue:

Screenshot 2024-01-29 at 20 16 22

I'll leave this open for now and see if I can hunt down the question itself (it's simply pulled down from the API and used in-app; no copy is stored locally, so I an't view the underlying data), but this seems less of an issue now.

@davep
Copy link
Contributor Author

davep commented Jan 29, 2024

Found it. And if I copy the question from the web view and paste it into a scratch buffer in Emacs:

Screenshot 2024-01-29 at 20 22 42

Inspecting what it is:

Screenshot 2024-01-29 at 20 24 08

@davep
Copy link
Contributor Author

davep commented Jan 29, 2024

And, just for final confirmation, plugging that into the test code above:

Screenshot 2024-01-29 at 20 28 55

@willmcgugan
Copy link
Collaborator

Good catch. Soft hyphens. Zero length, they indicate where the word is permitted to wrap.

Rich is reporting them as having a length of 1, where they should have a cell length of 0.

@davep
Copy link
Contributor Author

davep commented Jan 29, 2024

We should make @darrenburns's day by plugging this into soft-wrapped TextArea. 😏

@davep davep changed the title Rendering issue with long/wrapped text in OptionList Rendering issue with soft hyphens (~~long/wrapped text in OptionList~~) Jan 29, 2024
@willmcgugan
Copy link
Collaborator

I think this may be fixable from Rich. If you tackle this, ask @willmcgugan for an overview of how Rich calculates cell lengths.

@rodrigogiraoserrao rodrigogiraoserrao self-assigned this Feb 26, 2024
rodrigogiraoserrao added a commit to Textualize/rich that referenced this issue Feb 26, 2024
Bug Textualize/textual#4073 showed that the version of wcwidth we are using incorrectly reports the soft hyphen (codepoint 173, 0xAD) has having length 1.
rodrigogiraoserrao added a commit to Textualize/rich that referenced this issue Feb 26, 2024
Bug Textualize/textual#4073 showed that the version of wcwidth we are using incorrectly reports the soft hyphen (codepoint 173, 0xAD) has having length 1.
@rodrigogiraoserrao
Copy link
Contributor

@willmcgugan as discussed, it was enough to update wcwidth. See Textualize/rich#3289.

@rodrigogiraoserrao
Copy link
Contributor

@willmcgugan for this to be fixed in Textual we need a Rich release and then we need to update the Rich dependency in Textual.
Do you have a Rich release planned?

@willmcgugan
Copy link
Collaborator

Can do a Rich release shortly.

Can I just confirm that you have tested Textual with the update to Rich, and it fixes this issue?

@rodrigogiraoserrao
Copy link
Contributor

Yes. I installed rich locally and this fixed the issue in Textual.

@willmcgugan
Copy link
Collaborator

Pushed Rich 13.7.1

rodrigogiraoserrao added a commit that referenced this issue Feb 28, 2024
Rich 13.7.1 introduces the fix for #4073.
@rodrigogiraoserrao rodrigogiraoserrao linked a pull request Feb 28, 2024 that will close this issue
@rodrigogiraoserrao
Copy link
Contributor

Since we won't be pinning rich to 13.7.1, this can be closed already.

Copy link

github-actions bot commented Mar 5, 2024

Don't forget to star the repository!

Follow @textualizeio for Textual updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working question Further information is requested Task
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants