Skip to content

Commit

Permalink
update max width for all KeyboardHelpDialogs (HelpContent); remove ve…
Browse files Browse the repository at this point in the history 
…rticalIconSpacing, HelpContent.labelWithIcon.labelFirst; see phetsims/friction#158
  • Loading branch information
@zepumph
zepumph committed Jan 23, 2019
1 parent 570952c commit 25ad5ba
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 44 deletions.
2 changes: 1 addition & 1 deletion js/keyboard/TextKeyNode.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ define( function( require ) {
// text options
font: new PhetFont( { size: 12 } ),
fill: 'black',
textMaxWidth: 50,
textMaxWidth: 35, // Long keys like Space, Enter, Tab, Shift are all smaller than this.

// by default, key should tightly surround the text, with a bit more horizontal space
xPadding: 8
Expand Down
7 changes: 2 additions & 5 deletions js/keyboard/help/GeneralNavigationHelpContent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ define( function( require ) {

// constants
var DEFAULT_LABEL_OPTIONS = {
font: HelpContent.DEFAULT_LABEL_FONT,
maxWidth: HelpContent.DEFAULT_TEXT_MAX_WIDTH
font: HelpContent.DEFAULT_LABEL_FONT
};

/**
Expand All @@ -50,9 +49,7 @@ define( function( require ) {
options = _.extend( {
withGroupContent: false, // if true, the help content will include information about how to interact with groups

verticalIconSpacing: HelpContent.DEFAULT_VERTICAL_ICON_SPACING,

// passed to label Text
// passed to RichText label
labelOptions: null
}, options );

Expand Down
112 changes: 79 additions & 33 deletions js/keyboard/help/HelpContent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ define( function( require ) {
// heading defaults
const DEFAULT_HEADING_CONTENT_SPACING = 10; // spacing between h
const DEFAULT_HEADING_FONT = new PhetFont( { size: 16, weight: 'bold' } );
const DEFAULT_HEADING_MAX_WIDTH = 300; // i18n

// ratio 250:175 based on the old default maxWidth values between the heading and the labels
// see https://github.com/phetsims/friction/issues/158
const HEADING_MAX_WIDTH_SCALAR = 1.43;

// Content spacing and alignment
const DEFAULT_ALIGN = 'left'; // default alignment for the content and title
Expand All @@ -65,12 +68,11 @@ define( function( require ) {
* @constructor
*
* @param {string} headingString - the translatable label for this content
* @param {Array.<Object>} content - {label: <Node>, icon: <Node> }, icons and labels are each placed in their own
* VBox, and these layout boxes are aligned horizontally. It is assumed that each
* label and icon have bounds so that each row of content is aligned as desired.
* See HelpContent.labelWithIcon and HelpContent.labelWithIconList for how this is
* done with AlignBox.
*
* @param {Array.<HelpContentRow>} content - icons and labels are each placed in their own
* VBox, and these layout boxes are aligned horizontally. It is assumed that each
* label and icon have bounds so that each row of content is aligned as desired.
* See HelpContent.labelWithIcon and HelpContent.labelWithIconList for how this is
* done with AlignBox.
* @param {Object} [options]
*/
function HelpContent( headingString, content, options ) {
Expand All @@ -80,7 +82,10 @@ define( function( require ) {
// heading options
headingContentSpacing: DEFAULT_HEADING_CONTENT_SPACING,
headingFont: DEFAULT_HEADING_FONT,
headingMaxWidth: DEFAULT_HEADING_MAX_WIDTH,

// {number} The max width for all labels in the HelpContent. Used as the base sizing to layout the rest
// of the HelpContent.
baseLabelMaxWidth: DEFAULT_TEXT_MAX_WIDTH,

// VBox options
align: DEFAULT_ALIGN,
Expand All @@ -92,7 +97,7 @@ define( function( require ) {
// create the heading
var headingText = new Text( headingString, {
font: options.headingFont,
maxWidth: options.headingMaxWidth,
maxWidth: options.baseLabelMaxWidth * HEADING_MAX_WIDTH_SCALAR, // based off of the label max width

// a11y
tagName: 'h2',
Expand All @@ -103,8 +108,15 @@ define( function( require ) {
var icons = [];
var labels = [];
for ( var i = 0; i < content.length; i++ ) {
icons.push( content[ i ].icon );
labels.push( content[ i ].label );
const helpContentRow = content[ i ];

assert && assert( helpContentRow.text.maxWidth === null,
'HelpContent sets maxWidth for children' );

helpContentRow.text.maxWidth = options.baseLabelMaxWidth;

icons.push( helpContentRow.icon );
labels.push( helpContentRow.label );
}

var vBoxOptions = { align: 'left', spacing: DEFAULT_VERTICAL_ICON_SPACING };
Expand Down Expand Up @@ -147,36 +159,37 @@ define( function( require ) {
* @public
* @static
*
* @param {Node} label - label for the icon
* @param {Text|RichText} label - label for the icon
* @param {Node} icon
* @param {string} labelInnerContent - required to have the PDOM description of this row in the dialog
* @param {string} [labelInnerContent] - required to have the PDOM description of this row in the dialog
* @param {Object} [options]
* @returns {Object} - Object {label: <Node>, icon: <Node>} so HelpContent can layout content groups
* @returns {HelpContentRow} - so HelpContent can layout content groups
*/
labelWithIcon: function( label, icon, labelInnerContent, options ) {

options = _.extend( {
spacing: DEFAULT_LABEL_ICON_SPACING,
align: 'center',
labelFirst: true,
matchHorizontal: false,
iconOptions: {} // specific options for the icon mostly to add a11y content, extended with defaults below
}, options );
assert && assert( !options.children, 'children are not optional' );

assert && assert( !options.iconOptions.innerContent, 'should be specified as an argument' );
options.iconOptions = _.extend( {
tagName: 'li',
innerContent: labelInnerContent
}, options.iconOptions );

if ( labelInnerContent ) {
assert && assert( !options.iconOptions.innerContent, 'should be specified as an argument' );
options.iconOptions = _.extend( {
tagName: 'li',
innerContent: labelInnerContent
}, options.iconOptions );
}

// make the label and icon the same height so that they will align when we assemble help content group
var labelIconGroup = new AlignGroup( options );
var labelBox = labelIconGroup.createBox( label );
var iconBox = labelIconGroup.createBox( icon, options.iconOptions );

// options.children = options.labelFirst ? [ label, icon ] : [ icon, label ];
return options.labelFirst ? { label: labelBox, icon: iconBox } : { label: iconBox, icon: labelBox };
return new HelpContentRow( label, labelBox, iconBox );
},

/**
Expand All @@ -194,7 +207,7 @@ define( function( require ) {
* @param {string} labelInnerContent - content for the parallel DOM, read by a screen reader
* @param {Object} [options] - cannot pass in children
*
* @returns {Object} - Object {label: <Node>, icon: <Node>} so HelpContent can layout content groups
* @returns {HelpContentRow} - so HelpContent can layout content groups
*/
labelWithIconList: function( label, icons, labelInnerContent, options ) {

Expand Down Expand Up @@ -244,7 +257,7 @@ define( function( require ) {
var iconsBox = labelIconListGroup.createBox( iconsVBox, groupOptions ); // create the box to match height, but reference not necessary
var labelWithHeightBox = labelIconListGroup.createBox( labelBox, groupOptions );

return { label: labelWithHeightBox, icon: iconsBox };
return new HelpContentRow( label, labelWithHeightBox, iconsBox );
},

/**
Expand Down Expand Up @@ -456,7 +469,7 @@ define( function( require ) {
* stacked vertically in a Dialog. Loops through contentArray and finds the max x value of the left edge
* of the icon VBox. Then increases spacing of all other content HBoxes accordingly.
*
* @param {[].HelpContent} contentArray
* @param {HelpContent[]} contentArray
*/
alignHelpContentIcons: function( contentArray ) {

Expand All @@ -481,10 +494,17 @@ define( function( require ) {
* Convenience method to construct a help content for describing the grab button interaction
* @param {string} thingAsTitle - the item being grabbed, capitalized as a title
* @param {string} thingAsLowerCase - the item being grabbed, lower case as used in a sentence.
* @param {Object} [options]
* @static
* @returns {HelpContent}
*/
HelpContent.getGrabReleaseHelpContent = function( thingAsTitle, thingAsLowerCase ) {
HelpContent.getGrabReleaseHelpContent = function( thingAsTitle, thingAsLowerCase, options ) {

options = _.extend( {

// just a paragraph for this content, no list
a11yContentTagName: null
}, options );

// the visible heading string
var heading = StringUtils.fillIn( keyboardHelpDialogGrabOrReleaseHeadingPatternString, {
Expand All @@ -502,24 +522,50 @@ define( function( require ) {
} );

var label = new RichText( labelString, {
font: DEFAULT_LABEL_FONT,
maxWidth: DEFAULT_TEXT_MAX_WIDTH,
lineWrap: DEFAULT_TEXT_MAX_WIDTH
font: DEFAULT_LABEL_FONT
} );

var spaceKeyNode = new SpaceKeyNode();
var enterKeyNode = new EnterKeyNode();
var icons = HelpContent.iconOrIcon( spaceKeyNode, enterKeyNode );
var labelWithContent = HelpContent.labelWithIcon( label, icons, descriptionString, {
var labelWithContentRow = HelpContent.labelWithIcon( label, icons, descriptionString, {
iconOptions: {
tagName: 'p' // it is the only item so it is a p rather than an li
}
} );

return new HelpContent( heading, [ labelWithContent ], {
a11yContentTagName: null // just a paragraph for this content, no list
} );
return new HelpContent( heading, [ labelWithContentRow ], options );
};


/**
* Inner class POJO for keeping track of
* TODO: doc
*/
class HelpContentRow {

/**
* @param {Text|RichText} text - must be a child of the "label" Node
* @param {Node} label
* @param {Node} icon
*/
constructor( text, label, icon ) {


assert && assert( text instanceof Text || text instanceof RichText,
'unsupported label type: ' + text );

// assert && assert( label.hasChild( text ) )

// @public (read-only)
this.label = label;
this.icon = icon;
this.text = text;
}

}

return HelpContent;


} );
6 changes: 1 addition & 5 deletions js/keyboard/help/SliderControlsHelpContent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ define( function( require ) {

// constants
var DEFAULT_LABEL_OPTIONS = {
font: HelpContent.DEFAULT_LABEL_FONT,
maxWidth: HelpContent.DEFAULT_TEXT_MAX_WIDTH
font: HelpContent.DEFAULT_LABEL_FONT
};

/**
Expand All @@ -52,9 +51,6 @@ define( function( require ) {
// heading string for this content
headingString: keyboardHelpDialogSliderControlsString,

// icon options
verticalIconSpacing: HelpContent.DEFAULT_VERTICAL_ICON_SPACING,

// options passed to the label Text
labelOptions: null
}, options );
Expand Down

0 comments on commit 25ad5ba

Please sign in to comment.