EuiDualRange: Add possibility to set input properties for min and max inputs

[ffknob]

Summary

Closes #2737

Added possibility to set the input properties for the min and max inputs of EuiDualRange.

Checklist

• Check against all themes for compatibility in both light and dark modes
• Checked in mobile
• Checked in IE11 and Firefox
• Props have proper autodocs
• Added documentation examples
• Added or updated jest tests
• Checked for breaking changes and labeled appropriately
• Checked for accessibility including keyboard-only and screenreader modes
• A changelog entry exists and is marked appropriately

[elasticmachine]

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

[myasonik]

Not sure if this is the best API for this... Maybe we can do something like having a prop that takes an object and that whole object will be merged into the input?

I'm looking towards something more flexible because it's likely we'll want to be able to add other aria attributes to these inputs (such as aria-labelledby and aria-describedby) and it also unlocks some future capabilities like disabling only the min or max input while allowing the other to be used.

[ffknob]

ng towards something more flexible because it's likely we'll want to be able to add other aria attributes to these inputs

Do you mean instead of having an ariaLabels array with only that property, there should be an aria property, which would hold two objects, one for the min input and the other one for the max, each of which would allow any of the aria properties to be set... something like this?

EuiDualRange.propTypes = {
  ...
  aria: PropTypes.arrayOf(
    PropTypes.shape({
      'aria-label': PropTypes.string,
      'aria-labelledby': PropTypes.string,
      'aria-describedby': PropTypes.string,
    })
  ),
};

Then:

    const minInputAria = aria && aria[0] ? aria[0] : null;
    const maxInputAria = aria && aria[1] ? aria[1] : null;

And finally:

      <EuiRangeInput
       ...
        {...minInputAria}
      />

      <EuiRangeInput
       ...
        {...maxInputAria}
      />

(I'm not sure if this works or not, I'm just thinking out loud for now...)

[myasonik]

That's the direction I'm thinking but I would go even broader and not specify a shape. Something like:

EuiDualRange.propTypes = {
  ...
  minInputProps: PropTypes.object,
  maxInputProps: PropTypes.object
};

<EuiRangeInput
  ...
  {...minInputProps}
/>
<EuiRangeInput
  ...
  {...maxInputProps}
/>

This lets implementing devs pass in whatever props they need without EUI having to whitelist individual things.

[cchaos]

I agree with @myasonik that it should just be a generic ...rest style of object to pass to each input (which once TS'd can extend EuiRangeInput's type). Just be careful though that props like disabled and readOnly don't get overridden by the ...rest style prop.

[cchaos]

@ffknob Looks like you've got quite a few conflicts. Can you resolve those?

[thompsongl]

Sorry about the conflicts, @ffknob. I had a PR to convert these components to TypeScript that merged this morning; I didn't see your work here. Let me know if you want/need a hand with resolving the changes.

[ffknob]

Sorry about the conflicts, @ffknob. I had a PR to convert these components to TypeScript that merged this morning; I didn't see your work here. Let me know if you want/need a hand with resolving the changes.

No worries @thompsongl
I tried to simply...

  minInputProps?: InputHTMLAttributes<HTMLInputElement>;
  maxInputProps?: InputHTMLAttributes<HTMLInputElement>;

...but it gave me a lot of Typescript type errors with the EuiRangeInput's min,max, step and value properties.

I then solved this by creating the InputProps type... If you have a better suggestion please let me know.

[thompsongl]

@ffknob The interface to be used is EuiRangeInputProps. So import it from './range_input' on L15, and on then on L55-56:

minInputProps?: Partial<EuiRangeInputProps>;
maxInputProps?: Partial<EuiRangeInputProps>;

Just be careful though that props like disabled and readOnly don't get overridden by the ...rest style prop.

To the point made by @cchaos, this would allow consumers to clobber a lot of the props already being set (e.g., disabled). We could indicate in the docs that this prop is intended for use with aria attributes to dissuade from overwriting too much. Maintaining a specific list of props to allow or deny seems unmaintainable.

[cchaos]

@ffknob Sorry this one is a bit more complicated than it set out because it's not just about adding one prop anymore but extending those of the inputs but ensuring the component still applies the props it needs to. I've passed you a PR here: ffknob#2

[ffknob]

@cchaos thanks for leaving comments for each change, I always learn a lot with you guys

[cchaos]

LGTM once the CL conflict is resolved.

[thompsongl]

jenkins test this