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

Support: optional numbers. #86

Closed
oscbyspro opened this issue May 4, 2022 · 9 comments
Closed

Support: optional numbers. #86

oscbyspro opened this issue May 4, 2022 · 9 comments
Labels
enhancement improvements n' stuff
Milestone

Comments

@oscbyspro
Copy link
Owner

oscbyspro commented May 4, 2022

When an optional value is nil the view should show a placeholder. While supporting Optional<T> is orthogonal to number processing, and arguably does little to improve UX, it’s like a 0th class citizen in Swift-ville and people just expect it to work, even if SwiftUI.TextField doesn’t support it.

@oscbyspro oscbyspro added the enhancement improvements n' stuff label May 4, 2022
@oscbyspro oscbyspro added this to the v4.0.0 milestone May 4, 2022
@oscbyspro
Copy link
Owner Author

oscbyspro commented May 4, 2022

The easiest way to accomplish this is probably to create a new FormatStyle wrapper, then extend Optional to use it. It would have to modify the format and parse methods to consider optionality. Haven’t thought much about it yet, but I think it should be trivial to implement.

Edit: Formatting is trivial. Parsing is not trivial with current architecture.

@oscbyspro
Copy link
Owner Author

oscbyspro commented May 5, 2022

If bounds.autocorrect(_:) were moved to the end of the interpret(_:) method, then I it would be enough to allow Number.init to return an optional value (with a static isOptional, like isUnsigned and isInteger). Haven’t checked if moving it is sensible/valid, however. It’s a bit more complicated otherwise, because optionals are not comparable.

Edit: bounds.autocorrect(_:) cannot be moved elsewhere.

@oscbyspro
Copy link
Owner Author

oscbyspro commented May 5, 2022

As of dccf38d (dev) it should be sufficient to provide conditional behavior to the very first lines in interpret(_:) and merge(_:), after Number.init has been made optional. It’s not trivial, but it’s also not complicated and should not increase the module's complexity all that much.

Proposal

interpret(_:)
Let the wrapped format provide an optional commit per value. The commit should be nil unless the value itself is optional and nil, at which point the commit should contain it and an empty snapshot. If the commit exists, it should be returned.

merge(_:)
Optional values should return an empty commit if Number.init returns nil. Non-optional values should crash the application, because Number.init should never return nil for non-optional values.

@oscbyspro
Copy link
Owner Author

oscbyspro commented May 5, 2022

...forgot I still have to decide on how to handle that optional numbers are not comparable. Given that it has to be Comparable (or offer same functionality) it should be possible to delegate the proposed interpret(_:) specific part to Number.init a few lines below, reducing overall complexity. I should probably not use Comparable directly, however, considering that this part of the module is exposed as public.

@oscbyspro
Copy link
Owner Author

oscbyspro commented May 5, 2022

Proposal 2

Another option is to wrap NumberTextStyle as OptionalNumberTextStyle. It might be easier given how orthogonal Optional is. I say easier, because then I wouldn’t need to care about the fact optional numbers are not comparable. Hm.

Edit: This is better, each OptionalNumberTextStyle method can be written as a one-liner.

@oscbyspro
Copy link
Owner Author

oscbyspro commented May 6, 2022

Above solution works as expected. The text is interpreted as nil when there are no: integer digits, fraction separator, and no fraction digits. Otherwise it works exactly as the non-optional style. Signs are ignored because depending on format they may appear outside the user's input space, like "-$US 1,234,567.89" for example.

The only caveat with this solution is that I could not figure out how to type alias it as NumberTextStyle<Decimal?>. Instead, it had to be named OptionalNumberTextStyle<Decimal>, but most of the time it does not matter given that .number infers the correct style based on the value's type.

@oscbyspro
Copy link
Owner Author

oscbyspro commented May 6, 2022

It’s surprisingly uncomplicated and works well, but I need to make some adjustments to how invalid signs are handled on user input. As it stands, going from (nil) to (+1) in bounds (-9 to 0) does not work. It works for non-optional numbers because the sign is always valid, which means that discarding inputs with invalid signs is fine. This is not true for optional numbers, however. In this case it is expected that 1 would be interpreted as -1, because the input space is negative to nonpositive.

@oscbyspro
Copy link
Owner Author

Solution proposed in 97a97db (dev) and will be added in v4.0.0.

@oscbyspro
Copy link
Owner Author

oscbyspro commented May 6, 2022

Solved type aliasing in bec3dbe. It is now possible to write

NumberTextStyle<T> // _NumberTextStyle<T.NumberTextFormat>
T.NumberTextStyle  // _NumberTextStyle<T.NumberTextFormat>

NumberTextStyle<T>.Percent // _NumberTextStyle<T.NumberTextFormat.Percent>
T.NumberTextStyle .Percent // _NumberTextStyle<T.NumberTextFormat.Percent>

NumberTextStyle<T>.Currency // _NumberTextStyle<T.NumberTextFormat.Currency>
T.NumberTextStyle .Currency // _NumberTextStyle<T.NumberTextFormat.Currency>
NumberTextStyle<T?> // _OptionalNumberTextStyle<T.NumberTextFormat>
T?.NumberTextStyle  // _OptionalNumberTextStyle<T.NumberTextFormat>

NumberTextStyle<T?>.Percent // _OptionalNumberTextStyle<T.NumberTextFormat.Percent>
T?.NumberTextStyle .Percent // _OptionalNumberTextStyle<T.NumberTextFormat.Percent>

NumberTextStyle<T?>.Currency // _OptionalNumberTextStyle<T.NumberTextFormat.Currency>
T?.NumberTextStyle .Currency // _OptionalNumberTextStyle<T.NumberTextFormat.Currency>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement improvements n' stuff
Projects
None yet
Development

No branches or pull requests

1 participant