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

Panel minWidth mutability/shared minWidths #276

Closed
jonathanolson opened this issue Nov 22, 2016 · 18 comments
Closed

Panel minWidth mutability/shared minWidths #276

jonathanolson opened this issue Nov 22, 2016 · 18 comments

Comments

@jonathanolson
Copy link
Contributor

Currently, it's a bit clunky to properly align Panels (so that vertically stacked panels have the same width, etc.)

Something like this seems required:

var maxWidth = Math.max( content1.width, content2.width, content3.width );
var panel1 = new Panel( content1, { minWidth: maxWidth + xMargin * 2, ... } );
var panel2 = new Panel( content2, { minWidth: maxWidth + xMargin * 2, ... } );
var panel3 = new Panel( content3, { minWidth: maxWidth + xMargin * 2, ... } );

with the disadvantages:

  1. All content needs to be created and referenced first to determine the maximum content size. Especially annoying if one Panel is created in a supertype and another in a subtype that need to be overridden.
  2. Double margin needs to be added in, since minWidth includes margins.
  3. Can't change well dynamically.

I'd ideally like something like:

var panel1 = new Panel( content1, ... );
var panel2 = new Panel( content2, ... );
var panel3 = new Panel( content3, ... );

// after-creation layout
var maxWidth = Math.max( panel1.contentWidth, panel2.contentWidth, panel3.contentWidth );
panel1.contentWidth = maxWidth;
panel2.contentWidth = maxWidth;
panel3.contentWidth = maxWidth;

although to not have both a 'contentWidth' and 'minWidth' (which seems unclean), we'd presumably want a getter for "content width + margin" (which would potentially IGNORE any expansion from a previous minWidth), for example:

var maxWidth = Math.max( panel1.innerWidth, panel2.innerWidth, panel3.innerWidth );
panel1.minWidth = maxWidth;
// ...

Thoughts about how to handle this issue?

@samreid
Copy link
Member

samreid commented Dec 1, 2016

Brainstorming: what about SameWidthNode(others) which would be used for the content of the panels? You would still need to create contents first, but this could address margins and dynamic changes.

Usage:

var topPanelContents = ...;
var bottomPanelContents = ...;
var topPanel = new Panel(new SameWidthNode(topPanelContents,[bottomPanelContents]))
var bottomPanel = new Panel(new SameWidthNode(bottomPanelContents[topPanelContents]))

@pixelzoom
Copy link
Contributor

Short term: Along the lines of @samreid's suggestion (SameWidthNode), there are 2 others that I would find to be immediately useful: SameHeightNode, SameCenterNode.

Long term: I don't think that this type of layout responsibility belongs in Node subtypes. And putting these responsibilities in Node subtypes builds technical debt. A better solution would be something like Java's layout managers. GridBagLayout was generally the most useful. In other GUI toolkits (Motif,...) something like Java's SpringLayout was even more useful. Unfortunately Java's SpringLayout always seemed buggy, never really behaved as expected.

@samreid
Copy link
Member

samreid commented Dec 1, 2016

LayoutManagers sound reasonable, but it's a large area of uncertainty (how would the API look? how would it be implemented?). Can we implement something unintrusive, so that our explicit layouts which we commonly use (like setting panel1.left = panel2.left) still work perfectly?

@jonathanolson
Copy link
Contributor Author

I'll investigate options.

@samreid
Copy link
Member

samreid commented Dec 2, 2016

What about an option for HBox that tells the children to fill to max width?

@pixelzoom
Copy link
Contributor

What about an option for HBox that tells the children to fill to max width?

You mean min width, no?

@samreid
Copy link
Member

samreid commented Dec 2, 2016

"Set all of the children's width to be equal to the maximum width of the children."

@jonathanolson
Copy link
Contributor Author

How would an HBox option help with the described Panel case?

@samreid
Copy link
Member

samreid commented Dec 2, 2016

var panel1 = new Panel( content1, ... );
var panel2 = new Panel( content2, ... );
var panel3 = new Panel( content3, ... );
var panelBox = new VBox({sameWidth:true,children:[panel1,panel2,panel3]}) // or HBox
// Then VBox calls setters on the children to update their widths

EDIT: added the sameWidth option.
EDIT: replaced "does something magical" with "calls setters"

@jonathanolson
Copy link
Contributor Author

Changing the width of the children (Panels) with localBounds wouldn't resize the panels?

Ran into a case in ProportionPlayground where it would be ideal to have a same-sizing effect with alignment for each item (the ABSwitch needs to be logically centered, easy if we just say "left and right labels take up the same bounds size, but left is left-aligned and right is right-aligned". I'll plan to add a prototype that is capable of this behavior.

@samreid
Copy link
Member

samreid commented Dec 2, 2016

Changing the width of the children (Panels) with localBounds wouldn't resize the panels?

I didn't think it would, but I'm not too certain.

@jonathanolson
Copy link
Contributor Author

I've added AlignBox and AlignGroup as a proposal for how to handle these situations.

AlignBox takes a single content node, and allows the content to be aligned within a specified bounding box, with controls for horizontal/vertical alignment and margins on each side. Its local bounds will be that of the specified bounding box.

For example:

// our content
var circle = new scenery.Circle( 20 );

var box = new scenery.AlignBox( circle, {
  alignBounds: new dot.Bounds2( 0, 0, 100, 100 ), // specify the bounds inside which our content is aligned
  xAlign: 'center', // 'center' is the default for both xAlign and yAlign
  yAlign: 'top'
} );

box.bounds // Bounds2 {minX: 0, minY: 0, maxX: 100, maxY: 100}, equal to the alignBounds
circle.bounds // Bounds2 {minX: 30, minY: 0, maxX: 70, maxY: 40}, at the top-center of the alignBounds

margins can also be specified:

var box = new scenery.AlignBox( circle, {
  alignBounds: new dot.Bounds2( 0, 0, 100, 100 ),
  xAlign: 'center',
  yAlign: 'top',
  topMargin: 10
} );

circle.bounds // Bounds2 {minX: 30, minY: 10, maxX: 70, maxY: 50}, with the 10 offset at the top

and can be used without alignBounds (if alignBounds is not specified, content will be aligned in a bounding box with the upper-left of (0,0) and with a width/height matching the content + margins):

var box = new scenery.AlignBox( circle, {
  margin: 10,
  leftMargin: 20
} );

// fits the 40x40 circle with top/right/bottom margin of 10 and left margin of 20
box.bounds // Bounds2 {minX: 0, minY: 0, maxX: 70, maxY: 60}

// circle is inside the box's bounds with the specified margins
circle.bounds // Bounds2 {minX: 20, minY: 10, maxX: 60, maxY: 50}

AlignGroup groups together AlignBoxes, synchronizing their alignBounds so that they all have the same width, height or both (the default):

var circle = new scenery.Circle( 20 ); // 40x40
var rect = new scenery.Rectangle( 0, 0, 100, 20 ); // 100x20

var group = new scenery.AlignGroup();
var circleBox = new scenery.AlignBox( circle, {
  group: group
} );
var rectBox = new scenery.AlignBox( rect, {
  group: group
} );

// The bounds of both boxes fit both the circle and the rectangle
circleBox.bounds // Bounds2 {minX: 0, minY: 0, maxX: 100, maxY: 40}
rectBox.bounds // Bounds2 {minX: 0, minY: 0, maxX: 100, maxY: 40}

// The circle and rectangle are centered in their boxes (xAlign/yAlign default is 'center'):
circle.bounds // Bounds2 {minX: 30, minY: 0, maxX: 70, maxY: 40}
rect.bounds // Bounds2 {minX: 0, minY: 10, maxX: 100, maxY: 30}

Each AlignBox can specify its own alignment and margins:

var circleBox = new scenery.AlignBox( circle, {
  group: group,
  xAlign: 'right',
  yAlign: 'bottom'
} );
var rectBox = new scenery.AlignBox( rect, {
  group: group,
  xMargin: 20,
  yAlign: 'top'
} );

// Bounds for the boxes are larger to account for the rectangle's margin
circleBox.bounds // Bounds2 {minX: 0, minY: 0, maxX: 140, maxY: 40}
rectBox.bounds // Bounds2 {minX: 0, minY: 0, maxX: 140, maxY: 40}

circle.bounds // Bounds2 {minX: 100, minY: 0, maxX: 140, maxY: 40}, in the bottom-right (no margins)
rect.bounds // Bounds2 {minX: 20, minY: 0, maxX: 120, maxY: 20}, on the top, with horizontal margins

and AlignGroup can control individually whether width and height are matched between all boxes, or can vary:

var group = new scenery.AlignGroup( {
  matchVertical: false
} );

circleBox.bounds // Bounds2 {minX: 0, minY: 0, maxX: 100, maxY: 40}, height of 40 fits the circle exactly
rectBox.bounds // Bounds2 {minX: 0, minY: 0, maxX: 100, maxY: 20}, height of 20 fits the rectangle exactly

Potential usage examples:

  1. Given a bunch of size-varying icon graphics, create a bunch of same-size icon nodes.
  2. Keep Panels/AccordionBoxes/etc. aligned, so that their contents have the same width/height.
  3. Quickly add margins to a Node's bounds (simplier than struts, etc.)
  4. Can replace imperative layout logic based on bounds (e.g. keep something at the bottom-center of the ScreenView's visibleBounds with a specific offset from the bottom).

A few of these are exhibited in the Sun demo currently (top row has varying-size 'icons', bottom shows panel alignment).

It also avoids the need to know about all of the usages beforehand. For example, in Pendulum Lab, a supertype creates an AccordionBox with content, and one subtype creates a Panel that needs to be aligned. The supertype doesn't have to know about the subtype's panel contents BEFORE constructing the AccordionBox.

I'm curious about people's thoughts, as I've already found it helpful for my own code (I'll refactor my sim code as this changes, and can revert if it needs to be removed).

@jonathanolson
Copy link
Contributor Author

Retagging for developer meeting, as it would be good to get people's thoughts on the above comment (#276 (comment)) and currently-existing code (see Scenery's AlignBox/AlignGroup, they should be documented).

@pixelzoom
Copy link
Contributor

Code review issue: AlignBox is calling private functions of AlignGroup: onAlignBoxResized, removeAlignBox

@pixelzoom
Copy link
Contributor

12/8/16 dev meeting: Devs will test drive as they have time. I will try using this in unit-rates.

@jonathanolson
Copy link
Contributor Author

I'm sufficiently satisfied with this solution for sizing the content and alignment, so I'm fine personally with closing it. (Tagging for developer meeting to see if people have used it).

Sizing the container instead of the content is a whole other animal, and would be a separate issue.

@samreid
Copy link
Member

samreid commented Jul 18, 2017

I've had good a experience with AlignGroup in CCK.

@pixelzoom
Copy link
Contributor

Reviewed for developer meeting. I have had no experience with this yet, unlikely to be needed in the sim that I'm working on.

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

No branches or pull requests

3 participants