odd pink arc in control panel

pixelzoom commented 5 years ago

While responding to https://github.com/phetsims/projectile-motion/issues/197, I noticed an odd pink arc in over the word "Resistance", in this control panel in the Lab screen. This was in projectile-motion sha a2510ac7aa48287e8264b2f5805ef66238068381.

arouinfar commented 5 years ago

@pixelzoom the issue is that the Air Resistance checkbox should be left-aligned (as it is on other screens). The pink arc is the corresponding icon for air resistance. This is documented in the last checkbox in https://github.com/phetsims/projectile-motion/issues/177#issuecomment-540256334.

arouinfar commented 5 years ago

Thanks for reporting and finding the offending commit.

pixelzoom commented 5 years ago

Still a problem in master after pull-all.sh, projectile-motion df1a1fddd2954bb673f85572c35324bd9aee7897.

pixelzoom commented 5 years ago

@pixelzoom the issue is that the Air Resistance checkbox should be left-aligned (as it is on other screens).

It seems odd that this would cause the problem. If the icon is for the Air Resistance checkbox, it should be part of that checkbox's label (text + icon), and would move with the checkbox. So I think you should take a look at how this icon was added -- sounds like it was added incorrectly.

arouinfar commented 5 years ago

@pixelzoom currently the text and icons are separate nodes, but that is being addressed in #190

we can get rid of the checkboxAndIcon instrumentation, and just put the icon in the text Node given to checkbox.

pixelzoom commented 5 years ago

The icon is indeed being added incorrectly. Here's the code in LabProjectilePanel:

    // air resistance
    const airResistanceLabel = new Text( airResistanceString, LABEL_OPTIONS );
    const airResistanceCheckbox = new Checkbox( airResistanceLabel, model.airResistanceOnProperty, {
      maxWidth: options.minWidth - AIR_RESISTANCE_ICON.width - 3 * options.xMargin,
      boxWidth: 18,
      tandem: tandem.createTandem( 'airResistanceCheckbox' )
    } );
    const airResistanceCheckboxAndIcon = new HBox( {
      spacing: options.xMargin,
      children: [ airResistanceCheckbox, AIR_RESISTANCE_ICON ]
    } );

It should be:

    // air resistance
    const airResistanceCheckboxContent = new HBox( {
      spacing: options.xMargin,
      children: [ new Text( airResistanceString, LABEL_OPTIONS ), AIR_RESISTANCE_ICON ]
    } );
    const airResistanceCheckbox = new Checkbox( airResistanceCheckboxContent, model.airResistanceOnProperty, {
      maxWidth: options.minWidth - AIR_RESISTANCE_ICON.width - 3 * options.xMargin,
      boxWidth: 18,
      tandem: tandem.createTandem( 'airResistanceCheckbox' )
    } );

zepumph commented 5 years ago

I did a few fixes here. I'll explain them individually:

Surrounded the ICON in a containing Node, see https://github.com/phetsims/projectile-motion/issues/200
made the Panel VBox align left, though I couldn't

The icon is indeed being added incorrectly.

Agreed, and that was being fixed in many cases at the tail end of https://github.com/phetsims/projectile-motion/issues/190. For PhET-iO it was beneficial to able to have the icon in the checkbox. This was the last occurrence that I hadn't gotten to yet over in the last checkbox of https://github.com/phetsims/projectile-motion/issues/177#issuecomment-540256334.
I also fixed a bug where the lab screen combo box wasn't extending across the entire panel anymore, and I used that pattern in the intro screen too.

All of these changes were closely aligned because this layout issue occurred in during active development. It may happen again before work is done in this sim, and I will be on the lookout. See https://github.com/phetsims/projectile-motion/issues/177 for central issue of work for the repo.

phetsims / projectile-motion

odd pink arc in control panel #199