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

Draft: Add text wrap support for Label widget on Winform and GTK #2437

Closed
wants to merge 2 commits into from

Conversation

ikus060
Copy link

@ikus060 ikus060 commented Mar 1, 2024

This PR is a draft to start a conversation regarding the addition of text wrapping to Label widget. As this is a critical feature for my user interface, I wish to get this implemented.

I need help to implement the feature for alternative backend. I can provide implementation for Linux, Windows and MacOS, but my experience with smart phone are too limited to propose an implementation.

Regarding the implementation, it's clearly not in it's final state. I simply submit the PR to discuss the direction and provide a proof of concept.

The current state of the API seams a bit too limited to support a good implementation. That why I added a new function compute_size() to recompute the height of widget. Pack make calls to compute_size() to recompute the height of widget after fixing width. I think we could make the implementation of the compute_size function optional as very few widget required it.

Whats do you think ?

@mhsmith
Copy link
Member

mhsmith commented Mar 1, 2024

There's some discussion of this at #766 (comment).

Copy link
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

Summarising the discussion from #766 - before we get to an implementation, we need to settle on an interpretation. The biggest issue facing a text wrapping interpretation for labels is how that text wrapping interacts with the Pack layout mechanism. Until we've established that interpretation, discussing implementations is a little premature.

With that said - the implementation you've presented suggests that an implementation is possible; however, it seems to lean very heavily on GTK's native sizing mechanism. That may well produce a usable implementation for your use case, but we'll need to be able to reproduce that behaviour on every other platform, so we need to know what that behavior is, and have some idea for how it would be implemented on every other platform (even if we don't actually have a working implementation).

It looks like you've avoided some of the questions from #766 by making this an optional behavior; however, you've chosen to implement this as a flag to the constructor. It's not immediately clear to me that this is the right approach. This is an area where CSS has existing mechanisms to control line wrapping behavior, and one of the long-term goals of Toga is to replace Pack with CSS. To that end, it seems to me that this should be a style parameter, not a constructor argument.

From an architectural point of view, it's also not clear what compute_size() is actually computing, as compared to the intrinsic size that is already being computed by rehint(). I'm also not wild about the way this is a "if method exists" wart stuck to the side of one specific widget.

Finally, on an administrative note: this PR appears to be built on top of the PR you've submitted for #2435. We prefer to keep "one idea per PR", as it makes these specific changes a lot easier - as it stands, it's not necessarily clear which changes are related to text wrapping, and which are related to display and visibility handling.

@freakboy3742
Copy link
Member

Closing due to lack of response. If you intend to continue this work, let us know an we can re-open the PR.

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