From ad798b5eb01aa9e3a2dd415072d52d6350f43132 Mon Sep 17 00:00:00 2001
From: Titani <tlabaj@redaht.com>
Date: Mon, 25 Jan 2021 10:20:43 -0500
Subject: [PATCH] Updates from review comments

---
 .../src/components/Slider/Slider.tsx          |  41 +++---
 .../__snapshots__/Slider.test.tsx.snap        | 120 ++++++++++++------
 .../src/components/Slider/examples/Slider.md  |   6 +-
 3 files changed, 109 insertions(+), 58 deletions(-)

diff --git a/packages/react-core/src/components/Slider/Slider.tsx b/packages/react-core/src/components/Slider/Slider.tsx
index 074b23fe730..5ca67eb9a2b 100644
--- a/packages/react-core/src/components/Slider/Slider.tsx
+++ b/packages/react-core/src/components/Slider/Slider.tsx
@@ -3,6 +3,8 @@ import { useState } from 'react';
 import styles from '@patternfly/react-styles/css/components/Slider/slider';
 import { css } from '@patternfly/react-styles';
 import { SliderStep } from './SliderStep';
+import { InputGroup, InputGroupText } from '../InputGroup';
+import { TextInput } from '../TextInput';
 
 export interface SliderStepObject {
   /** Value of the step. This value is a percentage of the slider where the  tick is drawn. */
@@ -33,7 +35,7 @@ export interface SliderProps extends Omit<React.HTMLProps<HTMLDivElement>, 'onCh
   /** Label that is place after the input field */
   inputLabel?: string | number;
   /** Position of the input */
-  inputPosition?: 'aboveThumb' | 'left';
+  inputPosition?: 'aboveThumb' | 'right';
   /** Flag indicating input is disabled */
   isInputDisabled?: boolean;
   /** Value input callback.  Called when enter is hit while in input filed or focus shifts from input field */
@@ -54,11 +56,11 @@ export const Slider: React.FunctionComponent<SliderProps> = ({
   steps,
   isDiscrete = false,
   isInputVisible = false,
-  inputValue,
+  inputValue = 0,
   inputLabel,
   inputAriaLabel = 'Slider value input',
   thumbAriaLabel = 'Value',
-  inputPosition = 'left',
+  inputPosition = 'right',
   isInputDisabled,
   onChange,
   onValueChange,
@@ -85,8 +87,8 @@ export const Slider: React.FunctionComponent<SliderProps> = ({
 
   const style = { '--pf-c-slider--value': `${value}%` } as React.CSSProperties;
 
-  const onChangeHandler = (event: React.ChangeEvent<HTMLInputElement>) => {
-    setLocalInputValue(Number(event.currentTarget.value));
+  const onChangeHandler = (value: string) => {
+    setLocalInputValue(Number(value));
   };
 
   const handleKeyPressOnInput = (event: React.KeyboardEvent) => {
@@ -236,11 +238,11 @@ export const Slider: React.FunctionComponent<SliderProps> = ({
     }
   };
 
-  const inputGroup = (
-    <div className={css(styles.inputGroup)}>
-      <input
+  const displayInput = () => {
+    const textInput = (
+      <TextInput
         className={css(styles.formControl)}
-        disabled={isInputDisabled}
+        isDisabled={isInputDisabled}
         type="number"
         value={localInputValue}
         aria-label={inputAriaLabel}
@@ -250,9 +252,18 @@ export const Slider: React.FunctionComponent<SliderProps> = ({
         onFocus={onInputFocus}
         onBlur={onBlur}
       />
-      {inputLabel && <span className={css(styles.inputGroupText, 'pf-m-plain')}>{inputLabel}</span>}
-    </div>
-  );
+    );
+    if (inputLabel) {
+      return (
+        <InputGroup>
+          {textInput}
+          <InputGroupText className={css('pf-m-plain')}>{inputLabel}</InputGroupText>
+        </InputGroup>
+      );
+    } else {
+      return textInput;
+    }
+  };
 
   return (
     <div className={css(styles.slider, className)} style={style} {...props}>
@@ -289,12 +300,10 @@ export const Slider: React.FunctionComponent<SliderProps> = ({
           onClick={onThumbClick}
         />
         {isInputVisible && inputPosition === 'aboveThumb' && (
-          <div className={css(styles.sliderValue, styles.modifiers.floating)}>
-            <div className={css(styles.inputGroup)}>{inputGroup}</div>
-          </div>
+          <div className={css(styles.sliderValue, styles.modifiers.floating)}>{displayInput()}</div>
         )}
       </div>
-      {isInputVisible && inputPosition === 'left' && <div className={css(styles.sliderValue)}>{inputGroup}</div>}
+      {isInputVisible && inputPosition === 'right' && <div className={css(styles.sliderValue)}>{displayInput()}</div>}
       {rightActions && <div className={css(styles.sliderActions)}>{rightActions}</div>}
     </div>
   );
diff --git a/packages/react-core/src/components/Slider/_tests_/__snapshots__/Slider.test.tsx.snap b/packages/react-core/src/components/Slider/_tests_/__snapshots__/Slider.test.tsx.snap
index a82f790fe1d..9804555adc2 100644
--- a/packages/react-core/src/components/Slider/_tests_/__snapshots__/Slider.test.tsx.snap
+++ b/packages/react-core/src/components/Slider/_tests_/__snapshots__/Slider.test.tsx.snap
@@ -41,21 +41,51 @@ exports[`slider renders continuous slider 1`] = `
     <div
       className="pf-c-slider__value"
     >
-      <div
-        className="pf-c-input-group"
+      <TextInput
+        aria-label="Slider value input"
+        className="pf-c-form-control"
+        onBlur={[Function]}
+        onChange={[Function]}
+        onClick={[Function]}
+        onFocus={[Function]}
+        onKeyDown={[Function]}
+        type="number"
+        value={50}
       >
-        <input
+        <TextInputBase
           aria-label="Slider value input"
           className="pf-c-form-control"
+          innerRef={null}
+          isDisabled={false}
+          isLeftTruncated={false}
+          isReadOnly={false}
+          isRequired={false}
           onBlur={[Function]}
           onChange={[Function]}
           onClick={[Function]}
           onFocus={[Function]}
           onKeyDown={[Function]}
           type="number"
+          validated="default"
           value={50}
-        />
-      </div>
+        >
+          <input
+            aria-invalid={false}
+            aria-label="Slider value input"
+            className="pf-c-form-control pf-c-form-control"
+            disabled={false}
+            onBlur={[Function]}
+            onChange={[Function]}
+            onClick={[Function]}
+            onFocus={[Function]}
+            onKeyDown={[Function]}
+            readOnly={false}
+            required={false}
+            type="number"
+            value={50}
+          />
+        </TextInputBase>
+      </TextInput>
     </div>
   </div>
 </Slider>
@@ -286,30 +316,6 @@ exports[`slider renders slider with input 1`] = `
         tabIndex={0}
       />
     </div>
-    <div
-      className="pf-c-slider__value"
-    >
-      <div
-        className="pf-c-input-group"
-      >
-        <input
-          aria-label="Slider value input"
-          className="pf-c-form-control"
-          onBlur={[Function]}
-          onChange={[Function]}
-          onClick={[Function]}
-          onFocus={[Function]}
-          onKeyDown={[Function]}
-          type="number"
-          value={50}
-        />
-        <span
-          className="pf-c-input-group__text pf-m-plain"
-        >
-          %
-        </span>
-      </div>
-    </div>
   </div>
 </Slider>
 `;
@@ -356,13 +362,11 @@ exports[`slider renders slider with input above thumb 1`] = `
       <div
         className="pf-c-slider__value pf-m-floating"
       >
-        <div
-          className="pf-c-input-group"
-        >
+        <InputGroup>
           <div
             className="pf-c-input-group"
           >
-            <input
+            <TextInput
               aria-label="Slider value input"
               className="pf-c-form-control"
               onBlur={[Function]}
@@ -372,14 +376,52 @@ exports[`slider renders slider with input above thumb 1`] = `
               onKeyDown={[Function]}
               type="number"
               value={50}
-            />
-            <span
-              className="pf-c-input-group__text pf-m-plain"
             >
-              %
-            </span>
+              <TextInputBase
+                aria-label="Slider value input"
+                className="pf-c-form-control"
+                innerRef={null}
+                isDisabled={false}
+                isLeftTruncated={false}
+                isReadOnly={false}
+                isRequired={false}
+                onBlur={[Function]}
+                onChange={[Function]}
+                onClick={[Function]}
+                onFocus={[Function]}
+                onKeyDown={[Function]}
+                type="number"
+                validated="default"
+                value={50}
+              >
+                <input
+                  aria-invalid={false}
+                  aria-label="Slider value input"
+                  className="pf-c-form-control pf-c-form-control"
+                  disabled={false}
+                  onBlur={[Function]}
+                  onChange={[Function]}
+                  onClick={[Function]}
+                  onFocus={[Function]}
+                  onKeyDown={[Function]}
+                  readOnly={false}
+                  required={false}
+                  type="number"
+                  value={50}
+                />
+              </TextInputBase>
+            </TextInput>
+            <InputGroupText
+              className="pf-m-plain"
+            >
+              <span
+                className="pf-c-input-group__text pf-m-plain"
+              >
+                %
+              </span>
+            </InputGroupText>
           </div>
-        </div>
+        </InputGroup>
       </div>
     </div>
   </div>
diff --git a/packages/react-core/src/components/Slider/examples/Slider.md b/packages/react-core/src/components/Slider/examples/Slider.md
index 8da9453d279..804063ccbf2 100644
--- a/packages/react-core/src/components/Slider/examples/Slider.md
+++ b/packages/react-core/src/components/Slider/examples/Slider.md
@@ -48,7 +48,7 @@ class DiscreteInput extends React.Component {
 
   render() {
     const step = this.steps.find(step => step.value === this.state.value);
-    const displayValue = step ? step.label : undefined;
+    const displayValue = step ? step.label : 0;
     return (
       <>
         <Text component={TextVariants.h3}>Slider Value is: {displayValue}</Text>
@@ -138,7 +138,7 @@ class ValueInput extends React.Component {
 
     this.onValueChangeDiscrete = value => {
         const step = this.stepsDiscrete.find(step => step.value === value);
-        let inputValue = step ? step.label : undefined;
+        let inputValue = step ? step.label : 0;
         inputValue = Number(inputValue);
         this.setState({
           valueDiscrete: value,
@@ -148,7 +148,7 @@ class ValueInput extends React.Component {
 
     this.onValueChangePercent = value => {
         const step = this.stepsPercent.find(step => step.value === value);
-        let inputValue = step ? step.label.slice(0, -1) : undefined;
+        let inputValue = step ? step.label.slice(0, -1) : 0;
         inputValue = Number(inputValue);
         this.setState({
           valuePercent: value,