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

Screen icon design #28

Closed
pixelzoom opened this issue Dec 4, 2023 · 13 comments
Closed

Screen icon design #28

pixelzoom opened this issue Dec 4, 2023 · 13 comments

Comments

@pixelzoom
Copy link
Contributor

We'll need icons for the 5 screens, ideally (but not necessarily) something that works for both the Home screen and navigation bar.

Since there are 4 sims in this family, it would also be nice if the same screen icons are usable across all sims in which the screens appear.

Also preferred that the icons can be drawn in code, to make it possible to match color profile.

pixelzoom added a commit that referenced this issue Dec 21, 2023
@pixelzoom
Copy link
Contributor Author

pixelzoom commented Jan 9, 2024

We're getting close to feature complete, when it would be nice to have screen icons. So assigning to @arouinfar.

Before we put effort into artwork or implementation... It would be preferrable to create icons in code, so that color profiles can be supported. And it might be good to propose icons, and have the sim team approve. Or maybe brainstorm as part of a design meeting.

@arouinfar
Copy link
Contributor

Before we put effort into artwork or implementation... It would be preferrable to create icons in code

I agree @pixelzoom. It's also fewer assets to create and maintain.

And it might be good to propose icons, and have the sim team approve.

Sounds good, here's what I'm thinking:

  1. Bar Magnet
  1. Pickup Coil
  1. Electromagnet -- Hide the electrons and the slider
  1. Transformer -- Hide the electrons and the slider
  1. Generator -- I think the full generator has too much detail to put into the screen icon. Maybe just a closeup of the generator wheel without the rpm readout? We could consider adding a bit of the faucet like we do in the EFAC Systems screen icon

@pixelzoom feel free to tweak these ideas or propose any alternatives.

@arouinfar arouinfar assigned pixelzoom and unassigned arouinfar Jan 12, 2024
@pixelzoom
Copy link
Contributor Author

A few of these are not going to hold up very well in the navigation bar. We can certainly have different icons for the navigation bar, but it would be preferrable if we didn't. So I think we should consider more simplified, caricatured version of these things. @arouinfar let's discuss.

@pixelzoom
Copy link
Contributor Author

Here's a simplified set of icons, to add to the discussion. These are implemented in the sim, if you'd like to see them in context.

screenshot_2975 screenshot_2976

@pixelzoom
Copy link
Contributor Author

This probably needs to be discussed in a design meeting with the team.

@pixelzoom
Copy link
Contributor Author

1/25/2024 design meeting: @arouinfar @kathy-phet @ariel-phet @pixelzoom

Consensus was that the icons for the Bar Magnet and Generator screens are OK.

We did not reach any conclusions about the other screens. @ariel-phet commented that physicists will probably want to see coils in the icons.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Feb 6, 2024

... physicists will probably want to see coils in the icons.

Here's the code for creating a "coil" icon for the Pickup Screen (not committed):

function createScreenIcon
/**
 * Creates the icon for this screen.
 */
function createScreenIcon(): ScreenIcon {

  // Coil model
  const currentAmplitudeProperty = new NumberProperty( 0 );
  const currentAmplitudeRange = FELConstants.CURRENT_AMPLITUDE_RANGE;
  const coil = new Coil( currentAmplitudeProperty, currentAmplitudeRange, {
    numberOfLoopsRange: new RangeWithValue( 3, 3, 3 ),
    maxLoopArea: 7000,
    loopAreaPercentRange: new RangeWithValue( 100, 100, 100 ),
    tandem: Tandem.OPT_OUT
  } );
  coil.electronsVisibleProperty.value = false;

  // A hack because we have to have a subclass of FELMovable associated with the coil's background layer.
  const movable = new BarMagnet( {
    tandem: Tandem.OPT_OUT
  } );

  // Coil view
  const coilNode = new CoilNode( coil, movable, {
    tandem: Tandem.OPT_OUT
  } );
  
  // Disable interaction
  coilNode.pickable = false;

  // coilNode.backgroundNode needs to be added separately from coilNode.
  const icon = new Node( {
    children: [ coilNode.backgroundNode, coilNode ]
  } );

  return new ScreenIcon( icon, {
    fill: FELColors.screenBackgroundColorProperty,
    maxIconWidthProportion: 1,
    maxIconHeightProportion: 0.85
  } );
}

It looks passable (barely) on the Home screen. And it looks lousy in the navigation bar. See below.

screenshot_3017 screenshot_3015 screenshot_3016

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Feb 28, 2024

@arouinfar and I met about this, and did some brainstorming. Here are some changes that we're going to try...

Pickup Coil screen:

screenshot_3063

Transformer screen:

screenshot_3062

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Feb 29, 2024

The first screenshot below shows the lastest screen icons in context. For the Pickup Coil and Transformer screens, clipping off the tops of the wire ends on the coils looks decent.

The second screenshot below shows what happens when you leave the home screen, then come back to the home screen. The clipArea for the screen icons is no longer being applied. I've seen weirdness with clipArea + ScreenIcon before, but nothing like this. I'll need @jonathanolson's assistance -- see createScreenIcon in PickupCoilScreen and TransformerScreen. On macOS 14.3.1, it does not occur in Safari, but occurs in both Chrome 122 and Firefox 123.

screenshot_3066 screenshot_3067

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Feb 29, 2024

2/29/24 design meeting with @arouinfar @kathy-phet @ariel-phet @Nancy-Salpepi @pixelzoom

  • Icons look good.
  • @ariel-phet suggested margins for the Pickup Coil and Transformer icons, and centering the icons. I did a quick demo, everyone liked it, I will make it so.
  • clipArea problem still needs to be addressed, but @jonathanolson is on it.

@pixelzoom
Copy link
Contributor Author

Change requests have been completed. ScreenIcons were consolidated in FELScreenIconFactory.ts, so that I could factor out shared code (see createBarMagnetNode and createCoilNode).

I'll leave this issue open until the clipArea problem is resolved. The backup plan is to convert those icons to images using rasterize().

@pixelzoom
Copy link
Contributor Author

I've moved the clipArea bug to #89.

@arouinfar please review the screen icons. If everything looks OK, please close.

@arouinfar
Copy link
Contributor

Thanks @pixelzoom, looks good on main! Closing.

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

3 participants