mamaral / Neon

A powerful Swift programmatic UI layout framework.
MIT License
4.58k stars 389 forks source link

Allow setting xPad, yPad individually for groupAgainstEdge #19

Open brcolow opened 8 years ago

brcolow commented 8 years ago

Hello,

I am currently using the groupAgainstEdge method to group 3 dots together, aligned at the bottom edge, like so:

view.groupAgainstEdge(group: .Horizontal, views: [pageIndexCircles[0], pageIndexCircles[1], pageIndexCircles[2]], againstEdge: .Bottom, padding: 10, width: 10, height: 10)

The limitation is that the padding determines both the vertical padding from the bottom /and/ the horizontal padding of the dots together. E.g., if I increase the padding value, the dots move further away from the bottom edge and further away from each other. I assume this is a simple oversight in an otherwise amazing framework. Being able to specify the horizontal/vertical padding separately for groupAgainstEdge (where, depending on if the group is .Horizontal or .Vertical, the xPad will control the spacing between the elements of the group and yPad the distance from the edge, and vice-versa).

Thanks very much.

brcolow commented 8 years ago

To attempt to clarify: here is some code from NeonGroupable.swift:

case .Bottom:
    if group == .Horizontal {
        xOrigin = (self.width - (CGFloat(views.count) * width) - (CGFloat(views.count - 1) * padding)) / 2.0
        yOrigin = self.height - height - padding
        xAdjust = width + padding
    }

Here we see that the xOrigin and yOrigin use the same padding value - what I am asking is if it would be possible to split this into xPad and yPad, as it is in some other methods.

mamaral commented 8 years ago

I understand - sorry I've been extremely busy with my real-life work as well as have been sick, so I haven't done much open source work recently. I had originally done it that way so that the method signature wasn't any longer than it already was, but could add another function with the same name, but different params, that does what you're talking about, although that goes back to the complexity issue - Xcode autocompletes similarly-named methods with different signatures in a weird way making using the framework slightly more complicated/annoying. I can take a look, but to be honest don't know how soon this will happen.

brcolow commented 8 years ago

Sorry to hear that you have been sick :(. Here's the implementation of groupAgainstEdge that would allow one to specify both x and y padding individually so that we are both on the same page about what this issue is.

/// Tell a view to group an array of its subviews against one of its edges, specifying the padding between each subview
/// and their superview, as well as the size of each.
///
/// - parameters:
///   - group: The `Group` type specifying if the subviews will be laid out horizontally or vertically against the specified
/// edge.
///
///   - views: The array of views to grouped against the spcified edge. Depending on if the views are gouped horizontally
/// or vertically, they will be positioned in-order from left-to-right and top-to-bottom, respectively.
///
///   - againstEdge: The specified edge the views will be grouped against.
///
///   - xPad: If the `Group` type is horizontal, this is the padding to be applied between each of the subviews. Otherwise,
/// this is the padding between this group and its' superview.
/// 
///   - yPad: If the `Group` type is horizontal, this is the padding to be applied between this group and its' superview.
/// Otherwise, this is the padding to be applied between each of the subviews.
///
///   - width: The width of each subview.
///
///   - height: The height of each subview.
///
public func groupAgainstEdge(group group: Group, views: [Frameable], againstEdge edge: Edge, xPad: CGFloat, yPad: CGFloat, width: CGFloat, height: CGFloat) {
    if views.count == 0 {
        print("[NEON] Warning: No subviews provided to groupAgainstEdge().")
        return
    }

    var xOrigin : CGFloat = 0.0
    var yOrigin : CGFloat = 0.0
    var xAdjust : CGFloat = 0.0
    var yAdjust : CGFloat = 0.0

    switch edge {
    case .Top:
        if group == .Horizontal {
            xOrigin = (self.width - (CGFloat(views.count) * width) - (CGFloat(views.count - 1) * xPad)) / 2.0
            xAdjust = width + xPad
        } else {
            xOrigin = (self.width / 2.0) - (width / 2.0)
            yAdjust = height + yPad
        }

        yOrigin = yPad

    case .Left:
        if group == .Horizontal {
            yOrigin = (self.height / 2.0) - (height / 2.0)
            xAdjust = width + xPad
        } else {
            yOrigin = (self.height - (CGFloat(views.count) * height) - (CGFloat(views.count - 1) * yPad)) / 2.0
            yAdjust = height + yPad
        }

        xOrigin = xPad

    case .Bottom:
        if group == .Horizontal {
            xOrigin = (self.width - (CGFloat(views.count) * width) - (CGFloat(views.count - 1) * xPad)) / 2.0
            yOrigin = self.height - height - yPad
            xAdjust = width + xPad
        } else {
            xOrigin = (self.width / 2.0) - (width / 2.0)
            yOrigin = self.height - (CGFloat(views.count) * height) - (CGFloat(views.count) * yPad)
            yAdjust = height + yPad
        }

    case .Right:
        if group == .Horizontal {
            xOrigin = self.width - (CGFloat(views.count) * width) - (CGFloat(views.count) * xPad)
            yOrigin = (self.height / 2.0) - (height / 2.0)
            xAdjust = width + xPad
        } else {
            xOrigin = self.width - width - xPad
            yOrigin = (self.height - (CGFloat(views.count) * height) - (CGFloat(views.count - 1) * yPad)) / 2.0
            yAdjust = height + yPad
        }
    }

    for view in views {
        view.frame = CGRectMake(xOrigin, yOrigin, width, height)

        xOrigin += xAdjust
        yOrigin += yAdjust
    }
}

Likewise for groupAndFill:

/// Tell a view to group an array of its subviews filling the width and height of the superview, specifying the padding between
/// each subview and the superview.
///
/// - parameters:
///   - group: The `Group` type specifying if the subviews will be laid out horizontally or vertically.
///
///   - views: The array of views to be grouped against the sibling. Depending on if the views are grouped horizontally
/// or vertically, they will be positions in-order from left-to-right and top-to-bottom, respectively.
///
///   - xPad: If the `Group` type is horizontal, this is the padding to be applied between each of the subviews. Otherwise,
/// this is the padding between this group and its' superview.
///
///   - yPad: If the `Group` type is horizontal, this is the padding to be applied between this group and its' superview.
/// Otherwise, this is the padding to be applied between each of the subviews.
///
public func groupAndFill(group group: Group, views: [Frameable], xPad: CGFloat, yPad: CGFloat) {
    if views.count == 0 {
        print("[NEON] Warning: No subviews provided to groupAndFill().")
        return
    }

    var xOrigin : CGFloat = xPad
    var yOrigin : CGFloat = yPad
    var width : CGFloat = 0.0
    var height : CGFloat = 0.0
    var xAdjust : CGFloat = 0.0
    var yAdjust : CGFloat = 0.0

    switch group {
    case .Horizontal:
        width = (self.width - (CGFloat(views.count + 1) * xPad)) / CGFloat(views.count)
        height = self.height - (2 * yPad)
        xAdjust = width + xPad

    case .Vertical:
        width = self.width - (2 * xPad)
        height = (self.height - (CGFloat(views.count + 1) * yPad)) / CGFloat(views.count)
        yAdjust = height + yPad
    }

    for view in views {
        view.frame = CGRectMake(xOrigin, yOrigin, width, height)

        xOrigin += xAdjust
        yOrigin += yAdjust
    }
}
MontakOleg commented 7 years ago

Xcode autocompletes similarly-named methods with different signatures in a weird way making using the framework slightly more complicated/annoying

@mamaral what you mean about annoying? PureLayout actively uses method overloads and I have no problems with this.

autolayout

In the same time several of my colleagues don't using Neon because of lack of flexibility (and lacking of supporting right-to-left languages, but this is other story).

So, I think method overloads (with useful default values and more flexible params) would be great improvement of Neon api.

mamaral commented 7 years ago

@MontakOleg I agree. It's been a while since I was last in this discussion and honestly can't recall exactly what I meant before. I'm open to specific suggestions or even better would be pull requests as I am extremely busy with other work and likely won't be making changes in the near future.