Terra-core: Create enumeration for PropTypes.oneOf

[benbcai]

Issue Description

For any package that has a prop with a type of PropTypes.oneOf, an enumerations should be made available for that prop to make it easier for consumers to know the acceptable values. This also minimizes consumers from making typos when setting these props.

An example of of a prop using enumeration is alert type in the Alert component.

Issue Type

• [ ] New Feature
• [ ] Enhancement
• [ ] Bug
• [X] Other

[benbcai]

Packages that need updated:

• [ ] terra-arrange
• [ ] terra-base
  
  
  
  • Props need updated to reflect available options.
  
  
• [ ] terra-badge
• [ ] terra-button
• [ ] terra-button-group
• [ ] terra-collapsible-menu-view
• [ ] terra-form-textarea
• [ ] terra-grid
• [ ] terra-heading
• [ ] terra-i18n
  
  
  
  • Props need updated to reflect available options.
  
  
• [ ] terra-image
• [ ] terra-menu
• [ ] terra-modal-manager
• [ ] terra-overlay
• [ ] terra-slide-panel
• [ ] terra-progress-bar
• [ ] terra-popup
• [ ] terra-responsive-element
• [ ] terra-signature
• [ ] terra-spacer
• [ ] terra-text
• [ ] terra-table

[noahbenham]

When declaring the PropType, it might make more sense to use Object.values() instead of listing the options to improve maintainability.
For example, type: PropTypes.oneOf(Object.values(AlertTypes)).

[noahbenham]

Question for the team: Would it make more sense to export options directly instead of as part of Opts?

For example,

Internally

const AlertTypes = {
  ALERT: 'alert',
  ...
};

would become

export const AlertTypes = {
  ALERT: 'alert',
  ...
};

Consumers

import Alert from 'terra-alert';

<Alert type={Alert.Opts.Types.WARNING}>
  This is a warning!
</Alert>

could become

import Alert, { AlertTypes } from 'terra-alert';

<Alert type={AlertTypes.WARNING}>
  This is a warning!
</Alert>

This also allows consumers flexibility while still achieving our goal of typed props. For example,

import Alert, { AlertTypes as TerraAlertTypes } from 'terra-alert';
...

Motivation

Not only does this reduce object property expansion, it also allows us internally to reuse types in subcomponents with greater ease.
For example, to implement text style types for ItemDisplay and ItemDisplay.Comment, I'd need to do something like

ItemDisplay.jsx

import ItemComment from './ItemComment';

ItemDisplay.Comment = ItemComment;
ItemDisplay.Opts = {};
ItemDisplay.Opts.Style = TextStyles;

ItemComment.jsx

import ItemDisplay from './ItemDisplay';

ItemComment.Opts = {};
ItemComment.Opts.Style = ItemDisplay.Opts.Style;

This creates circular dependency issues and generally feels like a bad pattern.

[mjhenkes]

I'm against exporting enumerations of props. Using a string is more concise and lowers the complexity of maintaining and creating these components. With prop type warning you will be warned if you've had a spelling mistake or pass in a non supported type.

A mistake using the enum is more likely to cause a run time exception and crash the page.

For consumers it's simply this:

import Alert from 'terra-alert';

<Alert type="warning">
  This is a warning!
</Alert>

[dylanklohr]

@noahbenham, personally I prefer doing something like

const ButtonVariants = {
...
};

const ButtonTypes = {
...
};

export default Button;
export {  Types: ButtonTypes, Variants: ButtonVariants };

and in the case of something like button, where Opts has already been introduced, we'd leave the Opts export as well for passivity until the next major version update.

Thoughts?

[noahbenham]

I'd support that Dylan, although it sounds like Matt is advocating just using strings over an exported type.. Which makes sense to me.

[bjankord]

Plan to move forward with keeping prop-types as strings or as numbers. Doc examples should show strings used.

Export enums as below:

const ButtonVariants = {
...
};

const ButtonTypes = {
...
};

export default Button;
export {  ButtonTypes, ButtonVariants };