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

button content not centered with flat appearance strategy #235

Closed
pixelzoom opened this issue Apr 13, 2016 · 7 comments
Closed

button content not centered with flat appearance strategy #235

pixelzoom opened this issue Apr 13, 2016 · 7 comments
Assignees
Labels

Comments

@pixelzoom
Copy link
Contributor

Discovered in #234, starting with comment #234 (comment).

Here's a typical ResetButton with its default 3D appearance strategy, looks great:

screenshot_04

Here's the same button with flat appearance strategy. It looks wrong, the icon is shifted up and to the left, due to this bit of code that assumes 3D appearance strategy:

// The arrow shape doesn't look right when perfectly centered, account for that here,
// and see docs in RoundButtonView. The multiplier values were empirically determined.
xContentOffset: 0.03 * BUTTON_RADIUS,
yContentOffset: -0.0125 * BUTTON_RADIUS,

screenshot_05

Here's the same button again, flat appearance strategy, xContentOffset:0 and yContentOffset:0. Vertically centered, but shifted to the left because of the arrow head.

screenshot_05

Fix this by pairing the ResetShape with a transparent circle that has the proper radius to encompass the entire shape (including arrow head). Then use that as the icon, so that it centers correctly. That will look good for flat buttons.

3D buttons will need to have xContentOffset and yContentOffset tweaked further. And the issue of how to tweak xContentOffset and yContentOffset for 3D appearance should not be addressed specifically for this button. That should be handled generally in #234, by compensating in RectangularButtonView.flatAppearanceStrategy and RoundButtonView.flatAppearanceStrategy.

@pixelzoom
Copy link
Contributor Author

Other buttons that are compensating for 3D and will look wrong in flat:

scenery-phet.LeftRightSpinner
scenery-phet.StepButton
scenery-phet.StepBackButton
energy-skate-park-basics.ControlPointUI.deleteButton

@pixelzoom pixelzoom changed the title ResetButton looks wrong with flat appearance strategy buttons looks wrong with flat appearance strategy Apr 13, 2016
@pixelzoom pixelzoom changed the title buttons looks wrong with flat appearance strategy button content not centered with flat appearance strategy Apr 13, 2016
@pixelzoom
Copy link
Contributor Author

This should technically be in scenery-phet, but at this point I'm not going to move it. And there is sun work to be done here.

@pixelzoom
Copy link
Contributor Author

I will work on this when I have time, maybe with @jbphet's help.

pixelzoom added a commit to phetsims/scenery-phet that referenced this issue Apr 14, 2016
@pixelzoom
Copy link
Contributor Author

pixelzoom commented Apr 14, 2016

Resolved this in ResetButton by adjusting the localBounds of the icon like this:

// icon, with bounds adjusted so that center of circle appears to be centered on button, see sun#235
var resetIcon = new Path( new ResetShape( options.radius ), { fill: options.arrowColor } );
var reflectedIcon = new Path( resetIcon.shape.transformed( Matrix3.scaling( -1, 1 ) ) );
resetIcon.localBounds = resetIcon.localBounds.union( reflectedIcon.localBounds )

A flat button with xContentOffset:0 and yContentOffset:0 now looks properly centered. I also adjusted the default value of xContentOffset so that the button appears identical to how it did previously with 3D appearance strategy.

@pixelzoom
Copy link
Contributor Author

Moving the issue of offset compensation for 3D to #236.

@jbphet please review the changes I made to ResetButton in phetsims/scenery-phet@aae06db. Then you can close this issue.

@pixelzoom pixelzoom assigned jbphet and unassigned pixelzoom Apr 14, 2016
@pixelzoom
Copy link
Contributor Author

pixelzoom commented Aug 30, 2016

@jbphet Still awaiting your review of this. It's come up again in plinko-probability, so I'd like to put this issue to reset.

@jbphet
Copy link
Contributor

jbphet commented Aug 30, 2016

Just to be clear, what I think I'm reviewing here is the way that the bounds of the reset button icon were adjusted by overlaying a "reflection" so that it would be possible to center the button on its own without requiring compensation. This looks good to me, and we will potentially want to apply a similar approach to other icons. What this buys us is the ability to use such icons on flat buttons without requiring position compensation.

This solution does not actually move the position of the reset icon such that it looks good on a flat button - it just makes it so that if the offset values are overridden and set to 0, the icon does appear centered (I tried this). So I'm closing this, and I think all vested parties know that there is still work to be done on the general issue, which is being tracked in #236.

@pixelzoom - please reopen if you feel that I've missed the point on this.

@jbphet jbphet closed this as completed Aug 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants