OneForm icon indicating copy to clipboard operation
OneForm copied to clipboard

`translateProps` function feature proposal

Open souldreamer opened this issue 2 years ago • 3 comments

Was wondering if it wouldn't help to send the default computed child props to the translateProps function as one of the parameters. Most of the time you only need to change the behavior of one or two of the child properties, so copy/pasting the entire code to generate the properties seems wasteful.

The proposed code change would be in Field.jsx:

const DEFAULT_TRANSLATE_PROPS = ({defaultProps}) => defaultProps;

const Field = ({
  children,
  isCheckboxElement = false,
  isMultipleElement = false,
  translateProps = (
    DEFAULT_TRANSLATE_PROPS
  ),
}) => {
  const childProps = useMemo(
    () =>
      translateProps({
        errorMessages,
        fieldName,
        isChecked,
        isVisited,
        updateFieldValue,
        value,
        visitField,
        defaultProps: isHtmlElement
          ? {
              checked: isChecked,
              'data-error': errorMessages.length > 0 ? true : null,
              'data-visited': isVisited ? true : null,
              name: fieldName,
              onBlur: visitField,
              onChange: updateFieldValue,
              value,
            }
          : {
              checked: isChecked,
              dirty: isVisited,
              error: Boolean(errorMessages[0]),
              errorMessages: errorMessages,
              errors: errorMessages,
              isChecked,
              isDirty: isVisited,
              isTouched: isVisited,
              isVisited,
              name: fieldName,
              onBlur: visitField,
              onChange: updateFieldValue,
              touched: isVisited,
              value,
              visited: isVisited,
            },
      }),
    [errorMessages, fieldName, isChecked, isHtmlElement, isVisited, translateProps, updateFieldValue, value, visitField],
  );

A lot of the time you just need to change the behavior of onBlur and/or onChange and want to keep the other properties the same, and rewriting the same code is very non-DRY.

Another possibility is to do something like childProps = {...defaultProps, ...translateProps(...)} which would probably be even DRY-er, but at the cost of still passing on the properties the user might not want to pass, so that one would probably be a case of being too DRY.


While we're at it, passing the inputValue, isCheckbox, and isMultiple (though the latter two might be okay to be inferred because you'd be using a different translate function for them, but that's not always necessary, so having the info in the function would probably help in a few cases, and would definitely not hinder anything) into the translateProps function would probably not be amiss either (nor would passing all the other child props along), so we don't have to replicate logic from the <Field /> component and just use <Field />s with translateProps more.

While building a custom component isn't bad, re-using the <Field /> component should be preferred IMHO.

Also note that the current <MaterialUiField /> component nukes the helperText the user might set on the child component when there's no error. Its translateProps function should do something like helperText: errorMessages[0] || childProps.helperText.

Not to mention that the library having a <MaterialUiField /> is a bit redundant in its current form, at most you should have a materialUiTranslateProps export, or even better a tutorial in the docs where the code is shown and explained.


Also, hope I'm not bugging you too much with feature/change requests 😅

souldreamer avatar Sep 11 '21 17:09 souldreamer