Skip to content

Allow specifying xPad, yPad individually (ref #19)#21

Open
brcolow wants to merge 5 commits intomamaral:masterfrom
brcolow:master
Open

Allow specifying xPad, yPad individually (ref #19)#21
brcolow wants to merge 5 commits intomamaral:masterfrom
brcolow:master

Conversation

@brcolow
Copy link

@brcolow brcolow commented Nov 17, 2015

This is my attempt at addressing the issue I raised in #19. Obviously we should add some tests for the case where xPad != yPad, but I wanted to get this out there quickly for discussion/feedback.

Notes:

Depending on the specific combination of horizontal/vertical with top/left/bottom/right, the xPad and yPad values can both affect the resulting frame, or just one of them. Keeping track of which combinations allow for xPad and/or yPad to affect the resulting frame will allow us to (if desired) breakout methods by these groups so that no method parameter is redundant.

Which padding values affect the resulting frame:

groupAgainstEdge:

Horizontal:

  • Top: xPad, yPad
  • Left: xPad
  • Bottom: xPad, yPad
  • Right: xPad

Vertical:

  • Top: yPad
  • Left: xPad, yPad
  • Bottom: yPad
  • Right: xPad, yPad

brcolow and others added 5 commits November 16, 2015 18:09
For the group methods, there is two types of padding: between subviews and between the group and its sibling or superview. In all the grouping methods (except groupInCenter), we want to allow specifying the xPad and yPad values individually, so that the user can control both the padding between subviews and the padding between the group and its superview.
@brcolow
Copy link
Author

brcolow commented May 8, 2016

Ping @mamaral

@mamaral
Copy link
Owner

mamaral commented May 8, 2016

My first thoughts going over this is that if we wanted to go to this extent, why not make it fully customizable for all situations, i.e. allow them to specify xPad, yPad, and spacing? Then its less ambiguous as to what you're setting, and you can lay it out however you want?

@brcolow
Copy link
Author

brcolow commented May 8, 2016

Makes sense - thanks. I will be revising the PR soon. Thanks again for your feedback.

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.

2 participants