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 scale_effect modifier #830

Merged
merged 5 commits into from
Apr 25, 2023
Merged

Add scale_effect modifier #830

merged 5 commits into from
Apr 25, 2023

Conversation

dginsburg
Copy link
Contributor

@dginsburg dginsburg commented Apr 19, 2023

Closes #520

@supernintendo @carson-katri @shadowfacts Sorry to spam all of you on this. I'm not sure who is the best person for me to ask questions of. Can I get your thoughts on this approach to the Elixir parameters?

This PR is to implement the scale_effect modifier, which has two signatures.
https://developer.apple.com/documentation/swiftui/view/scaleeffect(_:anchor:)-pmi7
https://developer.apple.com/documentation/swiftui/view/scaleeffect(_:anchor:)-7q7as

I thought we could omit the signature that takes a single param because the version I've implemented that uses width and height can superseed the single param signature. I saw a similar decision made for aspect_ratio, even though the Swift version can accept a single float.

defmodule LiveViewNativeSwiftUi.Modifiers.AspectRatio do
use LiveViewNativePlatform.Modifier
modifier_schema "aspect_ratio" do
field :aspect_ratio, {:array, :float}, default: [1.0, 1.0]
field :content_mode, Ecto.Enum, values: ~w(fill fit)a
end
end

But for scale_effect, I don't think it's very clear to a non-Swift programmer that something called size or CGSize is actually a width and height scaling ratio. I've always felt that was an awkward Apple contrivance. For someone not familiar with Swift, width and height as individual params seem more obvious and simpler.

defmodule LiveViewNativeSwiftUi.Modifiers.ScaleEffect do
   use LiveViewNativePlatform.Modifier

   alias LiveViewNativeSwiftUi.Types.UnitPoint

   modifier_schema "scale_effect" do
     field :width, :float, default: 1.0
     field :height, :float, default: 1.0
     field :anchor, UnitPoint
   end
 end

What are your opinions? Do we want to always map as closely as possible to the SwiftUI function signatures or should we take some liberties that we think will make LVN more broadly understandable?

@shadowfacts
Copy link
Contributor

shadowfacts commented Apr 19, 2023

In this case, I think you could call it scale and still have the type be {:array, :float}. To me at least, that reads pretty naturally as [x scale, y scale].

Generally we want to stick fairly close to SwiftUI, but if there are cases where that hinders understandability, I think it's fine to depart.

@dginsburg dginsburg marked this pull request as ready for review April 20, 2023 19:46
@dginsburg dginsburg changed the title Adds scale_effect modifier Add scale_effect modifier Apr 21, 2023
@dginsburg dginsburg enabled auto-merge (squash) April 21, 2023 14:44
@dginsburg dginsburg merged commit 34a905f into main Apr 25, 2023
@carson-katri carson-katri deleted the 520-scale-effect branch April 25, 2023 13:17
carson-katri pushed a commit that referenced this pull request Apr 25, 2023
* Adds `scale_effect` modifier

* Specify scale as an array instead of width and height params

---------

Signed-off-by: Dylan Ginsburg <dylan@hey.com>
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.

Modifier -> SwiftUI -> Drawing and graphics: scaleEffect(_⚓)
3 participants