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

Add icon to mass panel #250

Closed
arouinfar opened this issue Mar 8, 2018 · 15 comments
Closed

Add icon to mass panel #250

arouinfar opened this issue Mar 8, 2018 · 15 comments

Comments

@arouinfar
Copy link

On the Lab screen, it may not be immediately clear which mass the slider is adjusting. After discussion with @ariel-phet we decided that adding an iconic mass to the panel would address this issue. @Denz1994 can you add an iconic adjustable mass to the left of the "Mass" title? Here's a mockup:

image

@Denz1994
Copy link
Contributor

Denz1994 commented Mar 8, 2018

One cute little mass icon coming up.

@Denz1994
Copy link
Contributor

Denz1994 commented Apr 4, 2018

@arouinfar Can you check this dev version for review?

@arouinfar
Copy link
Author

Those are indeed cute little mass icons! 😄

A few tweaks:

  • The mass looks a bit too small, can you increase the size ~10%?
  • There's also appears to be quite a bit of extra padding at the top of the panel. Can you trim those extra pixels off?
    image image

@arouinfar arouinfar assigned Denz1994 and unassigned arouinfar Apr 4, 2018
Denz1994 added a commit that referenced this issue Apr 11, 2018
@Denz1994
Copy link
Contributor

Panel changes were made on master @arouinfar?

@Denz1994 Denz1994 assigned arouinfar and unassigned Denz1994 Apr 11, 2018
@arouinfar
Copy link
Author

@Denz1994 it looks like trimming off the extra padding from the top of the panel shifted the icon down.
image

Can you center-align the icon with "Mass"?
image

@Denz1994
Copy link
Contributor

What about now @arouinfar? Try in master.

@Denz1994 Denz1994 assigned arouinfar and unassigned Denz1994 Apr 11, 2018
@arouinfar
Copy link
Author

Getting closer @Denz1994, but it's still not quite centered
image

@Denz1994
Copy link
Contributor

Sorry for the constant commits. Could you try master now @arouinfar?

@arouinfar
Copy link
Author

Mass icon looks great!

I've noticed that the "Mass" string looks way smaller than other slider titles. Can you bump it up to 14pt to match the others?

@arouinfar arouinfar removed their assignment Apr 11, 2018
@Denz1994
Copy link
Contributor

Denz1994 commented Apr 11, 2018

The mass string was smaller, due to the size of the panel it is contained in. I made the panel a bit larger so the string also increased in size. What do you think @arouinfar?

@arouinfar
Copy link
Author

arouinfar commented Apr 11, 2018

Sorry to keep kicking this back to you @Denz1994, but the font size still looks pretty small. Can you bump it up a bit more? It's okay if the panel ends up slightly larger.

@Denz1994
Copy link
Contributor

There was a problem with the overall layout of the contentNode of the panel. We needed to remove the force vectors from the mass icon. The icon created bounds to include them despite not being visible. This makes the adjustment of fonts and positioning the panel's content a lot less hacky.

Could you review the massValueControlPanel in master @arouinfar?

@arouinfar
Copy link
Author

Getting really close @Denz1994!! Alignment looks good, but the relative font sizes look a bit off. The min/max labels look too large, and the readout looks a bit too small. The min/max/readout should all be the same size, as in the Gravity slider (12 pt, I believe).

screen shot 2018-05-15 at 2 59 58 pm
screen shot 2018-05-15 at 2 59 58 pm copy

@arouinfar arouinfar removed their assignment May 15, 2018
@Denz1994
Copy link
Contributor

The max width on the readout was too constraining. It has been made larger and the fonts were not the same size for the min/max labels. Could you check in master @arouinfar?

@arouinfar
Copy link
Author

Looking good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants