phetsims / circuit-construction-kit-common

"Circuit Construction Kit: Basics" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
10 stars 10 forks source link

Circuit element NumberDisplays have inconsistent font size #822

Closed kathy-phet closed 1 year ago

kathy-phet commented 2 years ago

I was demoing this sim yesterday and saw a small inconsistency in font size. The Frequency number control is inconsistent with the other control sizes. I think would be nice if we could match that to the other sizes and do a MR if easy. @arouinfar - Can you take a peak? And let me know if this was intentional or we just overlooked it previously? Thx.
image

arouinfar commented 2 years ago

Summary:

Slack discussion:

Amy Rouinfar 9:31 AM It’s not just the frequency control. Resistance and Voltage NumberControls (which were the only controls in the 1.0 release of DC) are styled differently. They use a smaller font because we are spelling out volts and ohms (every other unit is abbreviated). Also, the NumberDisplay size is the same for both standard and high-V/high-R varieties so the font size is smaller due to the string lengths at max value. (edited) 9:34 With the work that SR is doing for iO, there won’t be a single resistance control shared by the resistor, bulb, high-R resistor, and high-R bulb, so we may have more freedom in the individual sizing.

Kathy Perkins 9:42 AM OK - I was thinking we just want the font size consistent. So if its smaller because it needs more space, we would just make them all match that same smaller size? Or make the text box bigger to fit the bigger size?

Amy Rouinfar 9:45 AM I agree, though I think the larger size is better and more readable when the sim is being projected. I wouldn’t bother with a MR since these NumberControls will be uncoupled for iO and we can pixel polish them for future release. 9:45 The inconsistencies in font have been around since CCK: DC 1.1 (when we added the fuse).

Kathy Perkins 9:49 AM OK. We will polish with PhET-iO release.

Sam Reid 9:49 AM @arouinfar can you please move or summarize this conversation in #822? 9:50 Please also mark #822 for the phet-io milestone

Amy Rouinfar 9:51 AM Yes, will do

Kathy Perkins 9:53 AM @arouinfar - I do agree the bigger size is more readable and preferrable if we can design that.

samreid commented 2 years ago

@arouinfar and I took a look and suspect this may have been caused by maxWidth restrictions. But we also observed that the maxWidth can extend over the tweakers if necessary (looks like it might be constrained just to the slider track). I'll take a look.

samreid commented 2 years ago

CircuitElementNumberControl specifies maxWidth at 2 levels:

    const options = merge( {
      titleNodeOptions: {
        maxWidth: CircuitElementNumberControl.NUMBER_CONTROL_ELEMENT_MAX_WIDTH,
        font: CCKCConstants.DEFAULT_FONT
      },
      numberDisplayOptions: {
        maxWidth: 80,
        valuePattern: valuePattern,
        decimalPlaces: numberOfDecimalPlaces,
        textOptions: {
          font: CCKCConstants.DEFAULT_FONT
        }
      },

The different-length text under the same maxWidths can yield different sizes of text. I'm not aware of how to match sizes across different components. Do we want to match the fonts only of things that can be seen at the same time, or of all editors within one screen, or all editors across all screens? Also, does this problem affect CCK DC?

arouinfar commented 2 years ago

Thanks @samreid. I'll tag this for design meeting.

There are a few things we can consider.

Also, does this problem affect CCK DC?

Yes, it does affect CCK: DC. The styling of the Current Rating NumberDisplay is ideal, but not so great for batteries, resistors, and bulbs.

matthew-blackman commented 1 year ago

12/15 Design Meeting We decided that this is worth about 15 minutes of work to try and make it look as uniform as possible. A robust solution is preferred, but this issue does not warrant a large overhaul of the UI logic.

samreid commented 1 year ago

Increasing the number display max width a little seemed to help a lot. After that change, the fuse looks like:

image

High voltage battery looks like:

image

stringTest=dynamic doubled

image

stringTest=double (to show when volts gets a lot longer)

image

quadrupled

image

x8

image

In AC

image

doubled

image

quadrupled

image

@arouinfar does this seem good enough?

arouinfar commented 1 year ago

Nice! This is definitely an improvement @samreid, thanks! I think this addresses the original issues, so I'll close.