Add common Styled System props to v6 components

[TyMick]

I'm running through and adding the common and (when appropriate) typography collections of Styled System props to components as per this comment on #378. Started with Accordion and I'm going to continue down alphabetically. 👍🏼

[TyMick]

Gave DatePicker a div wrapper in the process, to allow style props to apply to the whole thing. It was only "wrapped" in a React.Fragment before.

[TyMick]

DatePicker was also previously using colors defined in shared-styles instead of using the global Styled System theme, so I changed that as well while I was at it.

[RobertBolender]

Gave DatePicker a div wrapper in the process, to allow style props to apply to the whole thing. It was only "wrapped" in a React.Fragment before.

This is a kind of breaking change, please move this to a v6 version of the component.

[TyMick]

One change in c76ca22 (in date-period-picker/styled.jsx) caused a visual change in the component that it seems like was the original intent, just coded incorrectly before. Here's the change:

-import { colors, thickness, fonts } from '../shared-styles';
+import { themeGet } from '@styled-system/theme-get';
 
 ...
 
 export const DatePeriod = styled.div`
-	font: ${fonts.ui14};
+	${themeGet('textStyles.ui.14')}
 	...
 `;
 
 ...
 
 export const Label = styled.label`
 	...
-	font: ${fonts.ui14};
+	${themeGet('textStyles.ui.14')}
 	...
 `;

For context, here's fonts.ui14:

styled-ui/components/shared-styles/index.js

Lines 18 to 22 in 1e0c846

	ui14: `
	font-size: 14px;
	font-weight: normal;
	line-height: 14px;
	`,

And here's textStyles.ui.14:

styled-ui/theme/textStyles.js

Lines 89 to 93 in 1e0c846

	'14': {
	fontSize: '14px',
	lineHeight: '14px',
	fontWeight: fontWeights.normal,
	},

It's the same set of styles, but since the first version was written font: ${fonts.ui14}; instead of just ${fonts.ui14}, font-weight and line-height were set correctly, but not font-size, since that line ended up in the compiled CSS as font: font-size: 14px;.

Here's a before/after screenshot:

All this to say, should I leave this at the smaller ui.14 style which seems to have been the original intent, or should I increase it to ui.16 to match the previous actual font size (but also increase the line height by 2 pixels)?

[RobertBolender]

One change in c76ca22 (in date-period-picker/styled.jsx) caused a visual change in the component that it seems like was the original intent, just coded incorrectly before. Here's the change:

Solid catch.

All this to say, should I leave this at the smaller ui.14 style which seems to have been the original intent, or should I increase it to ui.16 to match the previous actual font size (but also increase the line height by 2 pixels)?

I'll check with our design team, but it can stay at ui.14 for now.

[TyMick]

I added propTypes to all the Dropdown components in my intent to add common style props to all of them, but once I did so, I found that they already each accepted common props by passing them to Box or Text or something similar.

One probably unexpected addition in baa4444, though, is a new utility function:

styled-ui/components/utils/prop-types.js

Lines 1 to 15 in 5e50f0d

	import PropTypes from 'prop-types';
	/**
	* Creates a prop-type-checking function that validates whether the prop value is an element of a
	* given component type.
	*
	* @param {React.ElementType} Component - The component type to check for.
	* @returns A PropTypes validation function to use in a component's `propTypes` object.
	*/
	export function elementOfType(Component) {
	// Not very intuitive, but this is the best way to check with PropTypes (see https://github.com/facebook/react/issues/2979)
	return PropTypes.shape({
	type: PropTypes.oneOf([Component]).isRequired,
	});
	}

I made it thinking it'd be nice to check component types of children on SegmentedButtonGroup and DropdownMenu. Here's how I'm using it:

styled-ui/components/button/Button.js

Lines 176 to 177 in 5e50f0d

	SegmentedButtonGroup.propTypes = {
	children: PropTypes.arrayOf(elementOfType(Button)).isRequired,

styled-ui/components/dropdown/dropdown-menu.jsx

Lines 69 to 81 in 5e50f0d

	DropdownMenu.propTypes = {
	children: PropTypes.arrayOf(
	PropTypes.oneOfType([
	elementOfType(MenuItem),
	elementOfType(MenuItemCheckbox),
	elementOfType(MenuItemLink),
	elementOfType(MenuItemSeparator),
	elementOfType(MenuItemIcon),
	elementOfType(MenuItemPrimaryText),
	elementOfType(MenuItemSecondaryText),
	elementOfType(MenuItemTitle),
	]),
	).isRequired,

Is that overkill and I should just make those PropTypes.node and call it good?

[RobertBolender]

Is that overkill and I should just make those PropTypes.node and call it good?

No objection to this.

I suppose one soft objection would be that this would start logging warnings for cases where I pass in a wrapped version of one of those components, wouldn't it?