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

Control panels do not horizontally align correctly when sensor tool icons removed from sensor toolbox #940

Closed
matthew-blackman opened this issue Jan 23, 2023 · 11 comments

Comments

@matthew-blackman
Copy link

When the voltmeter or ammeter icons are removed from the sensor toolbox via studio, the control panels do not align correctly. See screenshot below:

Screen Shot 2023-01-23 at 9 22 55 AM

Investigating with the scenery inspector, it looks like the 'advanced' options panel is aligning left within a layout component that is larger than the panel itself. See screenshot below:

Screen Shot 2023-01-23 at 9 25 24 AM

@AgustinVallejo
Copy link
Contributor

Will look into!

@matthew-blackman
Copy link
Author

matthew-blackman commented Jan 25, 2023

@AgustinVallejo @samreid and I made some progress on this. We found that if titleAlignX is set to 'center' in AdvancedAccordionBox, the panel alignment works but the title text is no longer left aligned. See below:

Screen Shot 2023-01-25 at 1 19 28 PM

This is not an ideal fix but it is getting us closer to the heart of the issue. Additionally, I found that there is an issue with the panel group width if the titleText string is long. See below:

Screen Shot 2023-01-25 at 1 18 33 PM

@jonathanolson
Copy link
Contributor

This looks like something I could help with, might be related to the layout engine?

@jonathanolson
Copy link
Contributor

Should be fixed with the above commits. It seems like code was added that changed the collapsedBox's bounds AFTER we used those bounds for the workaroundBox.

I also patched a case where the rectBounds of the workaroundBox would not have been getting properly updated.

@samreid
Copy link
Member

samreid commented Jan 25, 2023

Excellent, thanks @jonathanolson!

@samreid samreid self-assigned this Jan 25, 2023
@matthew-blackman
Copy link
Author

Things are looking great @jonathanolson thanks for looking into this!

One remaining issue is the width of the advanced accordion box when the title text is long. This is likely a CCK issue that I can look into. See below:

Screen Shot 2023-01-26 at 10 32 14 AM

@samreid
Copy link
Member

samreid commented Feb 2, 2023

Perhaps try to find a way to reduce the maxWidth of that accordion box title/label?

@marlitas
Copy link
Contributor

marlitas commented Feb 3, 2023

phetsims/sun#803 may be related to this issue.

@samreid samreid self-assigned this Feb 3, 2023
@samreid
Copy link
Member

samreid commented Feb 4, 2023

I believe the best long-term solution for this issue may involve a resizable accordion box, as described in phetsims/sun#803. For this release, it seems a very minor aesthetic issue in a corner case, and perhaps we should consider that, like related issue #939 (comment), this should not be a necessary feature for publishing this release.

@arouinfar OK to remove this issue from the milestone?

@arouinfar
Copy link
Contributor

it seems a very minor aesthetic issue in a corner case

I agree. The behavior has improved since originally reported, and the remaining issue really seems more like an edge case tom.

OK to remove this issue from the milestone?

Yes, untagging it now.

@arouinfar arouinfar removed this from the Full initial PhET-iO release: DC milestone Feb 6, 2023
@arouinfar arouinfar removed their assignment Feb 6, 2023
@samreid
Copy link
Member

samreid commented Feb 13, 2023

In today's meeting, @arouinfar @matthew-blackman and I agreed that this issue can be closed, because it is a minor cosmetic issue for this sim, and cannot be implemented until the accordion box is resizable in phetsims/sun#803. We suspect another sim will bump the priority of that issue, then we can re-evaluate in other sims as necessary.

@samreid samreid closed this as completed Feb 13, 2023
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

6 participants