yackle / CLImageEditor

MIT License
2.22k stars 574 forks source link

Added padding to menu items to centre align them #108

Closed fraserscottmorrison closed 9 years ago

fraserscottmorrison commented 9 years ago

The alignment of menu items has been bugging me for a while so Ive put together a pull request.

This change centre aligns the menu items by adding some padding to each frame's x coordinate, but only if there are 4 or 5 visible. If there <4 then they'll be left aligned as previous.

It takes into account the width of the screen so looks good on all iPhone screen widths

yackle commented 9 years ago

Thank you for your contribution. It seems like this code doesn't cover all screen width. I think it is better to modify the code, for example, as following:

  CGFloat diff = _menuView.frame.size.width - toolCount * W
  if (0<diff && diff<W) { // or 2*W ??
    padding = diff/(toolCount+1);
  }

How about you?

fraserscottmorrison commented 9 years ago

Yeah I like your solution better.

I’d go with diff<2*W as that will centre 4 icons on a iPhone6 - but its up to you

Cheers Fraser

On 8/06/2015, at 4:08 pm, Sho Yakushiji notifications@github.com wrote:

Thank you for your contribution. It seems like this code doesn't cover all screen width. I think it is better to modify the code, for example, as following:

CGFloat diff = _menuView.frame.size.width - toolCount * W if (0<diff && diff<W) { // or 2*W ?? padding = diff/(toolCount+1); } How about you?

— Reply to this email directly or view it on GitHub https://github.com/yackle/CLImageEditor/pull/108#issuecomment-109855320.

yackle commented 9 years ago

OK. Well then could you modify with 2*W and push?

fraserscottmorrison commented 9 years ago

Ive made that change. Do I need to redo the PR?

yackle commented 9 years ago

It's OK. Merged, thanks!

fraserscottmorrison commented 9 years ago

Thanks to you too!

On 8/06/2015, at 5:36 pm, Sho Yakushiji notifications@github.com wrote:

It's OK. Merged, thanks!

— Reply to this email directly or view it on GitHub.