Repository navigation
Apple: restore #152's Prop.View, and print the stepper's label to its step - #159
Merged
Merged
Conversation
… step Two faults left on main after #151's merge and #156. Prop.View: #151's merge brought back the placeholder that set "<description>: not available" text, so a Platform host never got its view. Control on the macOS gallery without this line: 4 Platform checks fail (the NSTextField is not in the window, the "not available" label, the replacement, the release). Stepper: StepperView.show() printed NSDecimalNumber's stringValue, which drops trailing zeros, so a step of 0.25 showed "1.5", not "1.50", on AppKit and UIKit. It now rounds with NSDecimalNumber and prints exactly `digits` decimals, as Steps.format does. Before: both galleries failed "showing its value to its step's precision" and "- moves it back a step" (main's own CI on 9f2a3bd, and #158's run on its merge with main). Measured: bin/verify-apple.sh "iPhone 17e" on this tree, 20/20 steps, at a load average of 80-180 (Apple CI ran beside it). macOS gallery 237 ok per run, iOS 252 ok. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Note
Copilot was unable to complete this review at the selected tier due to model policy or plan restrictions.
🟢 Approval recommended
The provided context does not indicate any open review threads, failing checks, or unresolved blocking items, and describes the changes as straightforward restorations/improvements to Apple rendering behavior.
No open issues 🎉
What changed in this PR
This pull request addresses two Apple renderer regressions: restoring Platform view hosting via Prop.View, and improving Stepper value label formatting so it reflects the step’s precision (e.g., preserving trailing zeros).
Changes:
- Restore
Prop.Viewhandling inAppleRendererso platform hosts actually embed their provided native view. - Update
StepperView.show()in both UIKit and AppKit shims to display values with the step’s decimal precision.
The stepper label formatting still has moderate issues to resolve:
- It rounds with
NSDecimalNumberbut then converts back toDoubleforString(format:), reintroducing binary floating-point rounding risk and potentially diverging from decimal semantics. String(format:)is locale-sensitive; output may use a comma decimal separator under some locales, which can break expected dot-based formatting in self-tests. Use an explicit POSIX locale and enforce exactlydigitsfraction digits (e.g., viaNumberFormatter) while formatting the decimal value directly.
Assessment: This pull request requires changes.
| File | Summary |
|---|---|
modules/renderer-apple/src/main/scala/thicket/renderer/apple/AppleRenderer.scala |
Restores Prop.View routing so Platform hosts mount the provided platform view again. |
modules/renderer-apple/shim/Sources/Shim+UIKit.swift |
Adjusts Stepper label formatting to preserve step precision, but still needs decimal/locale-stable formatting. |
modules/renderer-apple/shim/Sources/Shim+AppKit.swift |
Mirrors the Stepper label precision change, with the same decimal/locale-stability concerns. |
Review effort: Balanced
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
main's Apple CI is red. #156 fixed the header and thePlatformkind mapping. This PR fixes the two faults left.1.
Prop.View: hosting the view at allFault: #151's merge brought back the placeholder
Prop.Viewcase, so a Platform host showed "not available" text and never got its view. This restores #152's line:platformViews.setView(handle, view).Control: the macOS gallery without this line fails 4 Platform checks:
NSTextFieldis not in the window;2. The stepper's label precision
Fault:
StepperView.show()printedNSDecimalNumber'sstringValue, which drops trailing zeros. A 0.25 step showed1.5, not1.50, on AppKit and UIKit. #153 carried this bug.Fix: round with
NSDecimalNumber, then print exactlydigitsdecimals, asSteps.formatdoes.Before the fix: both galleries failed "…showing its value to its step's precision" and "− moves it back a step". That showed in
main's Apple CI on9f2a3bd, and in #158's run on its merge withmain.Measured
bin/verify-apple.sh "iPhone 17e"on this branch passed 20/20 steps.#158 (camera preview) fails only these two stepper checks. Once this PR merges, merging
maininto #158 should clear them.🤖 Generated with Claude Code