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

odd pink arc in control panel #199

Closed
pixelzoom opened this issue Oct 25, 2019 · 7 comments
Closed

odd pink arc in control panel #199

pixelzoom opened this issue Oct 25, 2019 · 7 comments
Assignees
Labels

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Oct 25, 2019

While responding to #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 a2510ac.

screenshot_1636

@arouinfar
Copy link
Contributor

@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 #177 (comment).

@arouinfar
Copy link
Contributor

Thanks for reporting and finding the offending commit.

@pixelzoom
Copy link
Contributor Author

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

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Oct 25, 2019

@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
Copy link
Contributor

@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
Copy link
Contributor Author

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
Copy link
Member

zepumph commented Oct 25, 2019

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

The icon is indeed being added incorrectly.

  • Agreed, and that was being fixed in many cases at the tail end of PhET-iO spec from client #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 Instrument for PhET-iO #177 (comment).
  • 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 #177 for central issue of work for the repo.

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

No branches or pull requests

3 participants