maxHeight property on Select Component is wrongly documented

[vaevictis]

Expectations

Following (this documentation)[https://garden.zendesk.com/react-components/select] I expected to be able to use maxHeight in one of these ways:
<Select maxHeight={'250'} /> or <Select maxHeight={250} />
Doc says:

One of type: string, number

Reality

These syntaxes get ignored by the component. The only one that works is <Select maxHeight={'250px'} />.

Steps to Reproduce

1. Try to implement one of the broken syntaxes
2. Witness that nothing happens
3. Use the other syntax
4. Admire that it works just fine

Implementation ideas:

• Fix Doc
  or
• Add a conversion layer somewhere that ensures the prop value is formatted in a way that's HTML/CSS compliant.

Fine Print

• Component: Select
• Browsers: Chrome (I suspect all)

[ryanseddon]

Thanks for the bug report @vaevictis 👍

A fix we could do it run parseFloat over maxHeight and either add or re-add the px value back e.g.

Change this:

react-components/packages/select/src/elements/Select.js

Lines 29 to 32 in cbd748e

	const StyledOverflowWrapper = styled.div`
	overflow-y: auto;
	max-height: ${({ maxHeight }) => maxHeight || 'inherit'};
	`;

To this:

const StyledOverflowWrapper = styled.div`
  overflow-y: auto;
  max-height: ${({ maxHeight }) => `${parseFloat(maxHeight)}px` || 'inherit'};
`;

Only downside is we enforce px only so if you were to pass em or any other unit it would change it to px.

This bug also affects Menu too.

[austingreendev]

I vote to just force the PropType to be a string.

If we start parsing ints/floats then we go down the path of forcing px values which may not be something we want to have everywhere.

[jzempel]

Agree with @Austin94. Let's not coerce pixels.

[austingreendev]

I'll open a quick PR for this.