-
Notifications
You must be signed in to change notification settings - Fork 12
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
layoutOptions is not working as expected for Separator. #1626
Labels
Comments
Looks like this should be an optionize3, |
jonathanolson
added a commit
that referenced
this issue
Apr 4, 2024
Fixed above, @pixelzoom can you review? |
This patch shows that this is not fixed. The accordion box shown in #1626 (comment) has no visible HSeparator at the top unless I also add Subject: [PATCH] rename PumpHandleDragListener for consistency with PumpHandleNode, https://github.com/phetsims/scenery-phet/issues/848
---
Index: js/common/view/EquationAccordionBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/EquationAccordionBox.ts b/js/common/view/EquationAccordionBox.ts
--- a/js/common/view/EquationAccordionBox.ts (revision 3597139b6d07b0840c8c1c3e69276123631422b5)
+++ b/js/common/view/EquationAccordionBox.ts (date 1712252607308)
@@ -84,8 +84,7 @@
new HSeparator( {
stroke: SEPARATOR_STROKE,
layoutOptions: {
- isSeparator: false,
- stretch: true
+ isSeparator: false
}
} ),
interactiveEquationNode, |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
For phetsims/graphing-lines#149, EquationAccordionBox has an HSeparator at the top of its content that is intended to be visible, for example in the published version:
Code for the content in EquationAccordionBox.ts:
In main, the top HSeparator is not visible. VBox/Flowbox, makes a separator invisible if there is not a visible Node on each side of the separator -- and this is expected.
But I also expected to be able to set
isSeparator: false
to make the top separator visible. This did not work -- there is no visible HSeparator:When I added the additional
stretch: true
, the HSeparator became visible:So there's something funny going on here with nested
layoutOptions
. In Separator.ts,DEFAULT_SEPARATOR_LAYOUT_OPTIONS
was added by @AgustinVallejo in e078be4, and its use looks suspicious, relevant code below. PerhapsDEFAULT_SEPARATOR_LAYOUT_OPTIONS
is getting overwritten? Oroptionize
is not working correctly for nested options?Assigning to @AgustinVallejo and @jonathanolson.
The text was updated successfully, but these errors were encountered: