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

Add SizeName property to label to control text size #5570

Merged
merged 4 commits into from
Mar 4, 2025

Conversation

dweymouth
Copy link
Contributor

@dweymouth dweymouth commented Feb 28, 2025

Description:

Allows for controlling the text size of a Label widget. Especially useful with selection support landing in 2.6 as the only other way to customize the size of selectable text would be a ThemeOverride container, which is heavyweight and verbose.

This mirrors the API that already exists for Hyperlink sizing.

Fixes #5561

Checklist:

  • Tests included.
  • Lint and formatter run with no errors.
  • Tests all pass.

Where applicable:

  • Public APIs match existing style and have Since: line.

@dweymouth dweymouth changed the title Add SizeName property to label Add SizeName property to label to control text size Feb 28, 2025
@coveralls
Copy link

coveralls commented Feb 28, 2025

Coverage Status

coverage: 62.313% (-0.09%) from 62.402%
when pulling f60004d on dweymouth:label-style
into 4cd00b5 on fyne-io:develop.

Copy link
Member

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

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

I think this really needs some tests, make sure the label is scaling appropriately, updates on change and that the selection follows it...

@dweymouth dweymouth requested a review from andydotxyz March 2, 2025 23:27
Copy link
Member

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

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

Thanks for adding a test.

I'm surprised that the images are the same size - does this indicate that the MinSize function is not updating, or that the test isn't respecting it?

I assume that a label with larger or smaller text would have a MinSize adjustment accordingly?

@dweymouth
Copy link
Contributor Author

Thanks for adding a test.

I'm surprised that the images are the same size - does this indicate that the MinSize function is not updating, or that the test isn't respecting it?

I assume that a label with larger or smaller text would have a MinSize adjustment accordingly?

The label's min size is larger, the test window just wasn't respecting it - pushed an update to the test to just use w.SetContent again to make sure the window expands to respect the new min size.

Copy link
Member

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

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

Sweet thanks

@dweymouth dweymouth merged commit df441d2 into fyne-io:develop Mar 4, 2025
11 checks passed
@dweymouth dweymouth deleted the label-style branch March 4, 2025 19:19
@ErikKalkoken
Copy link
Contributor

tyvm for implementing this feature!

sorry for my late comment (literally just saw this). But wouldn't it be better to limit the user to text-related sizes? e.g. by defaulting to normal size when a non-text related size is specified?

@dweymouth
Copy link
Contributor Author

tyvm for implementing this feature!

sorry for my late comment (literally just saw this). But wouldn't it be better to limit the user to text-related sizes? e.g. by defaulting to normal size when a non-text related size is specified?

you mean preventing something like theme.SizeNameInputRadius from being used? That's not possible, at least in a clean way, since both "text-related" and "non-text-related" size names have the same type, and are the same concept at the theme level. This also mirrors the API already on widget.Hyperlink for controlling size as well as what TextSegments in widget.RichText support

@ErikKalkoken
Copy link
Contributor

tyvm for implementing this feature!
sorry for my late comment (literally just saw this). But wouldn't it be better to limit the user to text-related sizes? e.g. by defaulting to normal size when a non-text related size is specified?

you mean preventing something like theme.SizeNameInputRadius from being used? That's not possible, at least in a clean way, since both "text-related" and "non-text-related" size names have the same type, and are the same concept at the theme level. This also mirrors the API already on widget.Hyperlink for controlling size as well as what TextSegments in widget.RichText support

ty for the clarification. did not realize that the RichText and Hyperlink API also allows it and in that regards it makes indeed more sense to keep it the same.

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.

4 participants