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

Sensor tool icons should reflow centered layout when others are made invisible #925

Closed
matthew-blackman opened this issue Jan 13, 2023 · 8 comments
Assignees

Comments

@matthew-blackman
Copy link

matthew-blackman commented Jan 13, 2023

If a sensor tool icon is removed via PhET-iO, the rest of the sensors should reflow so that they are center-aligned. This applied both within the sensor toolbox as well as the subset of ammeter types.

This issue was discovered in #917

@matthew-blackman matthew-blackman self-assigned this Jan 13, 2023
matthew-blackman pushed a commit that referenced this issue Jan 19, 2023
… remaining Panel sizing and alignment issues - see #925
matthew-blackman pushed a commit that referenced this issue Jan 19, 2023
… remaining Panel sizing and alignment issues - see #925
matthew-blackman pushed a commit to phetsims/circuit-construction-kit-dc that referenced this issue Jan 19, 2023
matthew-blackman pushed a commit to phetsims/circuit-construction-kit-dc that referenced this issue Jan 19, 2023
matthew-blackman pushed a commit that referenced this issue Jan 19, 2023
…ature correct sensors, regenerate API files - see #925
@matthew-blackman
Copy link
Author

Things are looking good in the sensor toolbox tree and ready for @arouinfar to review. Note that @samreid and I are aware of a panel alignment issue (see screenshot below) that is exposed by removing tool nodes, but is unrelated to this issue. I will open a separate issue for this.
Screen Shot 2023-01-19 at 2 31 17 PM

@arouinfar
Copy link
Contributor

@matthew-blackman looks good! Seems like the AccordionBox alignment hasn't been moved to its own issue yet, so I'll leave this one open so we don't forget.

@arouinfar arouinfar removed their assignment Jan 20, 2023
@matthew-blackman
Copy link
Author

Moved remaining panel layout issues to #940

@matthew-blackman matthew-blackman removed their assignment Jan 23, 2023
@matthew-blackman
Copy link
Author

While working on the featured overrides, @arouinfar and I noticed that the nonContactAmmeterToolNode does not have a visible property. It should have the same tree structure as the voltmeterToolNodes and seriesAmmeterToolNodes

@samreid
Copy link
Member

samreid commented Feb 3, 2023

I added that, and it seems to work well in testing. Closing.

@arouinfar
Copy link
Contributor

Looks like there's a regression and this is no longer fixed, as discovered in #987.

@matthew-blackman
Copy link
Author

The above commit appears to fix the issue for both DC and DC-VL. @arouinfar please confirm that all combos of voltmeter/ammeters visibility reflow correctly. @samreid if you can code review 2ff682b I think we should be able to MR this into CCK common.

@matthew-blackman
Copy link
Author

Commit 2ff682b causes a resurgence of #925.

@samreid @arouinfar and I agree that #925 should be a priority here, since it can appear during student interaction with sim. At this time, we are not going to put more effort into this layout issue. Closing.

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

No branches or pull requests

3 participants