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

Use Color.interpolateRGBA() for energy level colors in NucleonShellView #85

Closed
Luisav1 opened this issue Jul 6, 2023 · 9 comments
Closed

Comments

@Luisav1
Copy link
Contributor

Luisav1 commented Jul 6, 2023

Use Color.interpolateRGBA() where you set the stroke of energy levels in NucleonShellView instead of having a manually calculated array of a range of colors. Such that only the initial and final color properties of the energy levels are needed.
Change link on nucleonCountProperty to multilink that also listens to when the initial + final color properties change.

@zepumph
Copy link
Member

zepumph commented Aug 2, 2023

Working on this now.

@zepumph
Copy link
Member

zepumph commented Aug 2, 2023

  • Don't forget about Nucleon shell view

zepumph added a commit that referenced this issue Aug 2, 2023
…instead of colorGradientIndexNumberProperty), #85
zepumph added a commit to phetsims/shred that referenced this issue Aug 2, 2023
zepumph added a commit to phetsims/shred that referenced this issue Aug 2, 2023
zepumph added a commit to phetsims/shred that referenced this issue Aug 2, 2023
@zepumph
Copy link
Member

zepumph commented Aug 2, 2023

Ok Good progress on ParticleNode above, now onto NucleonShellView.

zepumph added a commit to phetsims/shred that referenced this issue Aug 2, 2023
zepumph added a commit that referenced this issue Aug 2, 2023
…nterpolate colors instead of hard coding the spectrum, #85
@zepumph
Copy link
Member

zepumph commented Aug 2, 2023

Ok. This has all been simplified. @Luisav1 will you please give these commits a spot check and feel free to close.

@zepumph zepumph removed their assignment Aug 2, 2023
Luisav1 added a commit that referenced this issue Aug 9, 2023
…PACITY constants are being used instead. See #85.
@Luisav1
Copy link
Contributor Author

Luisav1 commented Aug 9, 2023

Thanks @zepumph, this looks great. Closing.

@Luisav1
Copy link
Contributor Author

Luisav1 commented Aug 14, 2023

I noticed there's still a TODO pointing to this issue:

this.electronCloud.fill = new RadialGradient( 0, 0, 0, 0, 0, radius * factor )
// TODO: use color.interpolateRGBA() to use the same color property in both https://github.com/phetsims/build-a-nucleus/issues/85
.addColorStop( 0, BANColors.electronColorProperty.value.withAlpha( 200 ) )
.addColorStop( 0.9, BANColors.electronColorProperty.value.withAlpha( 0 ) );

This similar code is also in these two other places:

  • the initial creation of the electron cloud:
    fill: new RadialGradient( 0, 0, 0, 0, 0, MIN_ELECTRON_CLOUD_RADIUS )
    .addColorStop( 0, BANColors.electronColorProperty.value.withAlpha( 200 ) )
    .addColorStop( 0.9, BANColors.electronColorProperty.value.withAlpha( 0 ) )
  • the mini electron cloud creation
    fill: new RadialGradient( 0, 0, 0, 0, 0, 18 )
    .addColorStop( 0, BANColors.electronColorProperty.value.withAlpha( 200 ) )
    .addColorStop( 0.9, BANColors.electronColorProperty.value.withAlpha( 0 ) )

@Luisav1 Luisav1 reopened this Aug 14, 2023
@zepumph
Copy link
Member

zepumph commented Aug 14, 2023

@Luisav1 and I would like to ignore and delete the TODO, and the just factor out the new RadialGradient code into BANConstants, where the function takes a radius and returns the gradient. We aren't worried about creating multiple of the gradients, as we don't want to mutate the radius after creating a the Gradient. Also recreating these is fine for the Color Editor, and still allows you to see the changes if you want to update the electronColor

Luisav1 added a commit that referenced this issue Aug 15, 2023
Luisav1 added a commit that referenced this issue Aug 15, 2023
@Luisav1
Copy link
Contributor Author

Luisav1 commented Aug 16, 2023

The TODO was removed in the commit above and the RadialGradient is now created in BANConstants. Closing.

@Luisav1 Luisav1 closed this as completed Aug 16, 2023
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