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

Adjust 'center' method #14

Merged
merged 2 commits into from Oct 23, 2012

Conversation

Projects
None yet
2 participants
@ParkinT
Contributor

ParkinT commented Oct 5, 2012

Sorry it took so long to put this into a Pull Request.
I tried to follow your lead on the README formatting, but you may want to modify it.

Thom

ParkinT added some commits Oct 5, 2012

@ParkinT ParkinT referenced this pull request Oct 5, 2012

Closed

Adjust::Center #7

@colinta

This comment has been minimized.

Member

colinta commented Oct 5, 2012

Very cool!

Since format_frame outputs the entire frame, it seems like the verbose option is not crucial. Do you think it is important to keep?

@colinta

This comment has been minimized.

Member

colinta commented Oct 5, 2012

Just brainstorming, what do you think of this alternative argument order? My thinking is that controlling the x/y feature will be more common than the "n of m items" feature.

# center :xy  # => centers in the parent view
# center :x, 3, 3  # puts view in third-of-three position in the x direction
# center :y, 3, 1  # puts view in first-of-three position in the y direction
def center(sdirection='horizontal', element=1, total=1)
@ParkinT

This comment has been minimized.

Contributor

ParkinT commented Oct 6, 2012

I actually sweated over the order of the arguments; experimenting with several options.
In one case, I thought if you are like me and use "up arrow" to repeat commands a whole lot, the two parameters for "number of" should be last. In that way you can easily address a series of elements very quickly.
Then I struggled with the idea of what FEELS most intuitive.

I agree that the 'x/y' will be more often supplied.
Let me suggest that you make a few 'overloads' of this method. In that way we can "please all the people all the time"!!

@ParkinT

This comment has been minimized.

Contributor

ParkinT commented Oct 6, 2012

Regarding the 'verbose' option.
I agree that it seems (at first) to be superfluous. But, my intention is to provide "copy/paste" for the specific element you are centering.
In my work with it, I used mostly UIButton elements. I wanted to be able to simply copy/paste the origin.x, origin.y values.
I leave the final choice up to you. I believe this is one of those situations where there is no CORRECT way.
But you are best equipped to make sure we match the 'style' of the rest of the library.

@ParkinT

This comment has been minimized.

Contributor

ParkinT commented Oct 6, 2012

I had a thought overnight; regarding the parameters for the center method.
How about testing the first argument for is_hash?
A hash would allow the user to specify all the necessary detail easily. Now, I know a hash is impractical for a method like this where you are interacting on the command line. But if you then [pre]define a set of CONSTants that represent some of the most common setups, then the method will need only ONE parameter.
My thought was something like this

center HORIZONTAL
center VERTICAL
center 1OF3HORIZONTAL
center 2OF3HORIZONTAL
center 1OF2VERTICAL

In this case, those CONSTants will be defined as hashes. Do you follow my line of thinking?
As you said, I was just brainstorming.

Sorry for the string of comments :)

@colinta

This comment has been minimized.

Member

colinta commented Oct 19, 2012

Man, I went dark on this. Sorry! I have been way too busy recently. :-/

I'm not big on the constants - they can't start with a number anyway. But I like having some leeway on the arguments. My thinking goes like this:

  • a string or symbol is the direction - horizontal, vertical, or both.
  • two numbers is a location "index of total"
  • one number is the total, and index is 1
def center(*args)
  element = nil
  total = nil
  direction = 'h'
  args.each do |option|
    case option
    when String, Symbol  # accept string or symbol
      direction = option.to_s
    when Numeric
      if total
        element = total
      end
      total = option
    end
  end
  element = 1 unless element
  total = 1 unless total
  # ...
end
@ParkinT

This comment has been minimized.

Contributor

ParkinT commented Oct 20, 2012

Great idea(s).
I would argue that ENUMerators may be a better choice than CONSTants. But your approach is still very Ruby (expressive).

{I have been 'out of the loop' too. My Mom has been bouncing in and out of the hospital with [more] heart trouble.
I am traveling to be with her and my time has been too limited to focus on any of this lately.}

@colinta colinta merged commit b0b9305 into rubymotion:master Oct 23, 2012

@colinta

This comment has been minimized.

Member

colinta commented Oct 24, 2012

Ooph, family in the hospital is the worst. My heart goes out to you, Thom!

OK, now that I'm writing the announcement for SugarCube v0.11, I'm gonna change this method again at the last minute.

It feels weird that the first integer is the total, unless you provide two integers, and then it "becomes" the index. I'm gonna change that, so that the first integer is the total, the second is the index. Consistent and simple. I hope I haven't frankenstein-ed this into oblivion! :-)

center :x
center :x, 3
center :x, 3, 2
# or
center 3, 3, :xy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment