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

Constrain label measurements to the provided size constraint #449

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

kyleve
Copy link
Collaborator

@kyleve kyleve commented Apr 4, 2023

This fixes an on again off again issue we run into, where UILabel will return measurements larger than are provided to it. We constrain the measurement to the min of the measurement, or the input constraint.

@kyleve kyleve requested a review from a team as a code owner April 4, 2023 05:57
label.update(model: self, text: text, environment: environment, isMeasuring: true)
return label.sizeThatFits(constraint.maximum)

let size = label.sizeThatFits(constraint)
Copy link
Member

Choose a reason for hiding this comment

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

Does it work to use textRect(forBounds:limitedToNumberOfLines:) instead?

            return label.textRect(
                forBounds: .init(
                    origin: .zero,
                    size: constraint.maximum
                ),
                limitedToNumberOfLines: numberOfLines
            ).size

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So in general, we spent a LOT of time (in particular, @watt did) in early blueprint days testing the various ways to measure text properly, and sizeThatFits is really the only way to actually reproduce what UILabel does in all cases.

@n8chur n8chur requested a review from watt April 5, 2023 16:46
Copy link
Contributor

@nsillik nsillik left a comment

Choose a reason for hiding this comment

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

i thought i reviewed this a while ago, lgtm!

@robmaceachern
Copy link
Member

  • Do integration (at least before release)

…ller

* origin/main: (62 commits)
  AccessibilityBlocker aggressively blocking. (#483)
  Bumping version to 3.0.0 (#482)
  Allow for customization of the preview name (#478)
  Update CHANGELOG for AttributedLabel fixes (#480)
  Fix link detection for stretched labels (#476)
  chore: Updated minimum deployment target from iOS 14 to iOS 15 [UI-5185] (#479)
  chore(ios): Bump to Xcode 15.1 and Ruby 3.2.2 [UI-5184] (#477)
  AXCustomContent Support (#471)
  Bumping versions to 2.2.0 (#470)
  Update concatenation logic and unit tests
  update changelog
  never cache subelements
  optionally do not cache subelements
  Feature: add TintAdjustmentMode, modifiers, and tests
  Add to CHANGELOG
  Add tintAdjustmentMode to Image
  Bumping versions to 2.1.0 (#466)
  Resolved a Swift 5.9 compilation warning (#465)
  Update KeyboardObserver (#463)
  Bump activesupport from 7.0.4.3 to 7.0.7.2
  ...
@kyleve
Copy link
Collaborator Author

kyleve commented Mar 16, 2024

Running integration here: https://github.com/squareup/ios-register/pull/101927

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.

None yet

4 participants