From b55f4a13b27ff092f4a23beebb87c172ffe209a7 Mon Sep 17 00:00:00 2001
From: suomiy <suomiy@gmail.com>
Date: Wed, 20 Feb 2019 16:44:10 +0100
Subject: [PATCH] add units for disk size in VM disk page

change units to Gi
---
 sass/components/Form/form-group.scss          | 10 ++++
 src/components/CreateDiskRow/CreateDiskRow.js |  1 +
 .../fixtures/CreateDiskRow.fixture.js         |  3 +-
 .../__snapshots__/CreateDiskRow.test.js.snap  | 12 +++-
 .../CloneDialog/tests/CloneDialog.test.js     | 15 +++--
 src/components/Form/Checkbox.js               |  5 +-
 src/components/Form/Dropdown.js               |  7 ++-
 src/components/Form/FormFactory.js            | 31 ++++++++--
 src/components/Form/Integer.js                |  5 +-
 src/components/Form/Text.js                   |  5 +-
 .../__snapshots__/FormFactory.test.js.snap    | 60 ++++---------------
 .../Wizard/CreateVmWizard/strings.js          |  2 +-
 .../CreateVmWizard/tests/NetworksTab.test.js  |  4 +-
 .../__snapshots__/StorageTab.test.js.snap     |  2 +-
 14 files changed, 86 insertions(+), 76 deletions(-)

diff --git a/sass/components/Form/form-group.scss b/sass/components/Form/form-group.scss
index 1d4f87ede..bfc71d9e7 100644
--- a/sass/components/Form/form-group.scss
+++ b/sass/components/Form/form-group.scss
@@ -19,6 +19,16 @@
   margin-bottom: 23px;
 }
 
+.kubevirt-form-group__field--with-addendum {
+  width: calc(100% - 2.5em);
+  display: inline-block;
+}
+
+.kubevirt-form-group__addendum {
+  margin-left: 0.5em;
+  width: 2em;
+}
+
 .kubevirt-form-group__field-help .popover {
   max-width: 400px;
 }
diff --git a/src/components/CreateDiskRow/CreateDiskRow.js b/src/components/CreateDiskRow/CreateDiskRow.js
index e3602ec40..ab0e12347 100644
--- a/src/components/CreateDiskRow/CreateDiskRow.js
+++ b/src/components/CreateDiskRow/CreateDiskRow.js
@@ -47,6 +47,7 @@ const getDiskColumns = (storage, storageClasses, LoadingComponent) => {
       required: true,
       title: 'Size',
       disabled: storage.creating,
+      addendum: 'Gi',
     },
     bus: {
       id: 'disk-bus',
diff --git a/src/components/CreateDiskRow/fixtures/CreateDiskRow.fixture.js b/src/components/CreateDiskRow/fixtures/CreateDiskRow.fixture.js
index 1f2544107..0c6b43e10 100644
--- a/src/components/CreateDiskRow/fixtures/CreateDiskRow.fixture.js
+++ b/src/components/CreateDiskRow/fixtures/CreateDiskRow.fixture.js
@@ -1,4 +1,5 @@
 import { CreateDiskRow } from '../CreateDiskRow';
+import { storageClasses } from '../../Wizard/CreateVmWizard/fixtures/CreateVmWizard.fixture';
 
 export default {
   component: CreateDiskRow,
@@ -7,7 +8,7 @@ export default {
     onChange: () => {},
     onAccept: () => {},
     onCancel: () => {},
-    storageClasses: [],
+    storageClasses,
     LoadingComponent: () => null,
   },
 };
diff --git a/src/components/CreateDiskRow/tests/__snapshots__/CreateDiskRow.test.js.snap b/src/components/CreateDiskRow/tests/__snapshots__/CreateDiskRow.test.js.snap
index edcfedd86..2eee79512 100644
--- a/src/components/CreateDiskRow/tests/__snapshots__/CreateDiskRow.test.js.snap
+++ b/src/components/CreateDiskRow/tests/__snapshots__/CreateDiskRow.test.js.snap
@@ -26,6 +26,7 @@ exports[`<CreateDiskRow /> renders correctly 1`] = `
         "validate": [Function],
       },
       "size": Object {
+        "addendum": "Gi",
         "disabled": undefined,
         "id": "disk-size",
         "required": true,
@@ -33,9 +34,14 @@ exports[`<CreateDiskRow /> renders correctly 1`] = `
         "type": "positive-number",
       },
       "storageClass": Object {
-        "choices": Array [],
-        "defaultValue": "--- No Storage Class Available ---",
-        "disabled": true,
+        "choices": Array [
+          "nfs",
+          "iscsi",
+          "glusterfs",
+          "azuredisk",
+        ],
+        "defaultValue": "--- Select Storage Class ---",
+        "disabled": false,
         "id": "disk-storage-class",
         "type": "dropdown",
       },
diff --git a/src/components/Dialog/CloneDialog/tests/CloneDialog.test.js b/src/components/Dialog/CloneDialog/tests/CloneDialog.test.js
index 5de554a11..1d5671cf0 100644
--- a/src/components/Dialog/CloneDialog/tests/CloneDialog.test.js
+++ b/src/components/Dialog/CloneDialog/tests/CloneDialog.test.js
@@ -30,11 +30,10 @@ const startVm = (component, checked) => setCheckbox(component.find('#start-vm').
 
 const clickCloneVm = component => clickButton(component, 'Clone Virtual Machine');
 
-const getNameValidation = component =>
-  component
-    .find(HelpBlock)
-    .at(0)
-    .text();
+const getNameValidation = component => {
+  const wrapper = component.find(HelpBlock).at(0);
+  return wrapper.exists() ? wrapper.text() : null;
+};
 
 const selectNamespace = (component, namespace) => {
   const namespaceDropdown = component.find('#namespace-dropdown');
@@ -89,19 +88,19 @@ describe('<CloneDialog />', () => {
     };
 
     const component = mount(testCloneDialog([vm1, vm2]));
-    expect(getNameValidation(component)).toEqual('');
+    expect(getNameValidation(component)).toEqual(null);
 
     setVmName(component, getName(vm1));
     expect(getNameValidation(component)).toEqual(`Name ${VIRTUAL_MACHINE_EXISTS}`);
 
     selectNamespace(component, getNamespace(vm2));
-    expect(getNameValidation(component)).toEqual('');
+    expect(getNameValidation(component)).toEqual(null);
 
     setVmName(component, getName(vm2));
     expect(getNameValidation(component)).toEqual(`Name ${VIRTUAL_MACHINE_EXISTS}`);
 
     setVmName(component, 'othername');
-    expect(getNameValidation(component)).toEqual('');
+    expect(getNameValidation(component)).toEqual(null);
   });
 
   it('calls clone when clicked on finish', async () => {
diff --git a/src/components/Form/Checkbox.js b/src/components/Form/Checkbox.js
index ae9ae0dfb..a12e0bbd1 100644
--- a/src/components/Form/Checkbox.js
+++ b/src/components/Form/Checkbox.js
@@ -4,12 +4,13 @@ import { Checkbox as PfCheckbox, noop } from 'patternfly-react';
 
 import { checkboxHandler } from './utils';
 
-export const Checkbox = ({ id, title, disabled, checked, onChange, onBlur }) => (
+export const Checkbox = ({ id, title, disabled, checked, onChange, onBlur, className }) => (
   <PfCheckbox
     id={id}
     checked={checked}
     onBlur={checkboxHandler(onBlur)}
     onChange={checkboxHandler(onChange)}
+    className={className}
     disabled={disabled}
   >
     {title}
@@ -21,6 +22,7 @@ Checkbox.defaultProps = {
   checked: false,
   onChange: noop,
   onBlur: noop,
+  className: undefined,
   disabled: false,
 };
 
@@ -30,5 +32,6 @@ Checkbox.propTypes = {
   checked: PropTypes.bool,
   onChange: PropTypes.func,
   onBlur: PropTypes.func,
+  className: PropTypes.string,
   disabled: PropTypes.bool,
 };
diff --git a/src/components/Form/Dropdown.js b/src/components/Form/Dropdown.js
index c7ab9a861..a5f9b6de1 100644
--- a/src/components/Form/Dropdown.js
+++ b/src/components/Form/Dropdown.js
@@ -1,15 +1,16 @@
 import React from 'react';
 import PropTypes from 'prop-types';
+import classNames from 'classnames';
 import { ButtonGroup, DropdownButton, MenuItem, noop, Tooltip, OverlayTrigger } from 'patternfly-react';
 
-export const Dropdown = ({ id, value, disabled, onChange, choices, withTooltips }) => {
+export const Dropdown = ({ id, value, disabled, onChange, choices, className, withTooltips }) => {
   const title = typeof value === 'object' ? value.name || value.id : value;
   return (
     <ButtonGroup justified key={id}>
       <DropdownButton
         id={id}
         bsStyle="default"
-        className="kubevirt-dropdown"
+        className={classNames('kubevirt-dropdown', className)}
         title={title}
         disabled={disabled}
         onSelect={onChange}
@@ -44,6 +45,7 @@ Dropdown.defaultProps = {
   onChange: noop,
   disabled: false,
   choices: [],
+  className: undefined,
   withTooltips: false,
 };
 
@@ -53,5 +55,6 @@ Dropdown.propTypes = {
   choices: PropTypes.array,
   onChange: PropTypes.func,
   disabled: PropTypes.bool,
+  className: PropTypes.string,
   withTooltips: PropTypes.bool,
 };
diff --git a/src/components/Form/FormFactory.js b/src/components/Form/FormFactory.js
index fd5a35e9e..bef64c24f 100644
--- a/src/components/Form/FormFactory.js
+++ b/src/components/Form/FormFactory.js
@@ -36,8 +36,8 @@ export const getFormElement = props => {
           value={isControlled ? value || '' : undefined}
           defaultValue={isControlled ? undefined : defaultValue}
           onBlur={onBlur}
-          className={className}
           onChange={onChange}
+          className={className}
           disabled={disabled}
         />
       );
@@ -54,6 +54,7 @@ export const getFormElement = props => {
           checked={isControlled ? value || false : undefined}
           onBlur={onBlur}
           onChange={onChange}
+          className={className}
           disabled={disabled}
         />
       );
@@ -66,6 +67,7 @@ export const getFormElement = props => {
           defaultValue={isControlled ? undefined : defaultValue}
           onBlur={onBlur}
           onChange={onChange}
+          className={className}
           positive
           disabled={disabled}
         />
@@ -78,7 +80,14 @@ export const getFormElement = props => {
       );
     case CUSTOM:
       return (
-        <CustomComponent onChange={onChange} id={id} key={id} value={value || defaultValue} extraProps={extraProps} />
+        <CustomComponent
+          onChange={onChange}
+          id={id}
+          key={id}
+          value={value || defaultValue}
+          className={className}
+          extraProps={extraProps}
+        />
       );
     case PASSWORD:
       return (
@@ -90,6 +99,7 @@ export const getFormElement = props => {
           onBlur={onBlur}
           onChange={onChange}
           disabled={disabled}
+          className={className}
           type="password"
         />
       );
@@ -102,6 +112,7 @@ export const getFormElement = props => {
           defaultValue={isControlled ? undefined : defaultValue}
           onBlur={onBlur}
           onChange={onChange}
+          className={className}
           disabled={disabled}
         />
       );
@@ -167,17 +178,23 @@ const getFormGroups = ({ fields, fieldsValues, onFormChange, textPosition, label
       const values = fieldsValues[key];
       const validation = get(values, 'validation');
       const value = get(values, 'value');
-      const hasValidationMessage = !!get(validation, 'message');
+      const validationMessage = get(validation, 'message');
+      const hasValidationMessage = !!validationMessage;
+      const hasAddendum = !!field.addendum;
 
       const child = getFormElement({
         ...field,
         value,
         isControlled: true,
         onChange: newValue => onChange(fields, fieldsValues, newValue, key, onFormChange),
+        className: classNames(field.className, {
+          'kubevirt-form-group__field--with-addendum': hasAddendum,
+        }),
       });
 
-      const label = horizontal &&
-        field.title && (
+      let label;
+      if (horizontal && field.title) {
+        label = (
           <Col sm={labelSize} className={textPosition}>
             {field.type !== 'checkbox' && (
               <React.Fragment>
@@ -189,6 +206,7 @@ const getFormGroups = ({ fields, fieldsValues, onFormChange, textPosition, label
             )}
           </Col>
         );
+      }
 
       return (
         <FormGroup
@@ -203,7 +221,8 @@ const getFormGroups = ({ fields, fieldsValues, onFormChange, textPosition, label
           {label}
           <Col sm={controlSize}>
             {child}
-            <HelpBlock>{get(validation, 'message')}</HelpBlock>
+            {hasAddendum && <span className="kubevirt-form-group__addendum">{field.addendum}</span>}
+            {hasValidationMessage && <HelpBlock>{validationMessage}</HelpBlock>}
           </Col>
         </FormGroup>
       );
diff --git a/src/components/Form/Integer.js b/src/components/Form/Integer.js
index 6a08931b0..ce81c6fc1 100644
--- a/src/components/Form/Integer.js
+++ b/src/components/Form/Integer.js
@@ -56,7 +56,7 @@ const isInputValid = (allowedKeys, keyCode, targetValue, additionalValidation) =
   return false;
 };
 
-export const Integer = ({ id, value, disabled, defaultValue, onChange, onBlur, positive, nonNegative }) => {
+export const Integer = ({ id, value, disabled, defaultValue, onChange, onBlur, positive, nonNegative, className }) => {
   let allowedKeys;
   let validRegex;
   let fixAfterValue;
@@ -116,6 +116,7 @@ export const Integer = ({ id, value, disabled, defaultValue, onChange, onBlur, p
       defaultValue={defaultValue}
       onBlur={eventValueHandler(onBlur)}
       onChange={eventValueHandler(onChange)}
+      className={className}
       disabled={disabled}
     />
   );
@@ -128,6 +129,7 @@ Integer.defaultProps = {
   onBlur: noop,
   positive: false,
   nonNegative: false, // is ignored when positive == true
+  className: undefined,
   disabled: false,
 };
 
@@ -139,5 +141,6 @@ Integer.propTypes = {
   onBlur: PropTypes.func,
   positive: PropTypes.bool,
   nonNegative: PropTypes.bool,
+  className: PropTypes.string,
   disabled: PropTypes.bool,
 };
diff --git a/src/components/Form/Text.js b/src/components/Form/Text.js
index 7fe7e8d59..f7fb6917e 100644
--- a/src/components/Form/Text.js
+++ b/src/components/Form/Text.js
@@ -4,7 +4,7 @@ import { FormControl, noop } from 'patternfly-react';
 
 import { eventValueHandler } from './utils';
 
-export const Text = ({ id, value, disabled, defaultValue, onChange, onBlur, type }) => (
+export const Text = ({ id, value, disabled, defaultValue, onChange, onBlur, type, className }) => (
   <FormControl
     id={id}
     key={id}
@@ -13,6 +13,7 @@ export const Text = ({ id, value, disabled, defaultValue, onChange, onBlur, type
     defaultValue={defaultValue}
     onBlur={eventValueHandler(onBlur)}
     onChange={eventValueHandler(onChange)}
+    className={className}
     disabled={disabled}
   />
 );
@@ -23,6 +24,7 @@ Text.defaultProps = {
   onChange: noop,
   onBlur: noop,
   disabled: false,
+  className: undefined,
   type: 'text', // or password
 };
 
@@ -32,6 +34,7 @@ Text.propTypes = {
   defaultValue: PropTypes.string,
   onChange: PropTypes.func,
   onBlur: PropTypes.func,
+  className: PropTypes.string,
   disabled: PropTypes.bool,
   type: PropTypes.string,
 };
diff --git a/src/components/Form/tests/__snapshots__/FormFactory.test.js.snap b/src/components/Form/tests/__snapshots__/FormFactory.test.js.snap
index d3bfb50ba..6ed504a12 100644
--- a/src/components/Form/tests/__snapshots__/FormFactory.test.js.snap
+++ b/src/components/Form/tests/__snapshots__/FormFactory.test.js.snap
@@ -172,6 +172,7 @@ exports[`<FormFactory /> renders correctly 1`] = `
               className="col-sm-5"
             >
               <Text
+                className=""
                 disabled={false}
                 id="text"
                 key="text"
@@ -182,6 +183,7 @@ exports[`<FormFactory /> renders correctly 1`] = `
               >
                 <FormControl
                   bsClass="form-control"
+                  className=""
                   componentClass="input"
                   disabled={false}
                   id="text"
@@ -202,13 +204,6 @@ exports[`<FormFactory /> renders correctly 1`] = `
                   />
                 </FormControl>
               </Text>
-              <HelpBlock
-                bsClass="help-block"
-              >
-                <span
-                  className="help-block"
-                />
-              </HelpBlock>
             </div>
           </Col>
         </div>
@@ -253,6 +248,7 @@ exports[`<FormFactory /> renders correctly 1`] = `
               className="col-sm-5"
             >
               <Text
+                className=""
                 disabled={false}
                 id="password"
                 key="password"
@@ -263,6 +259,7 @@ exports[`<FormFactory /> renders correctly 1`] = `
               >
                 <FormControl
                   bsClass="form-control"
+                  className=""
                   componentClass="input"
                   disabled={false}
                   id="password"
@@ -283,13 +280,6 @@ exports[`<FormFactory /> renders correctly 1`] = `
                   />
                 </FormControl>
               </Text>
-              <HelpBlock
-                bsClass="help-block"
-              >
-                <span
-                  className="help-block"
-                />
-              </HelpBlock>
             </div>
           </Col>
         </div>
@@ -334,6 +324,7 @@ exports[`<FormFactory /> renders correctly 1`] = `
               className="col-sm-5"
             >
               <Text
+                className=""
                 disabled={false}
                 id="required"
                 key="required"
@@ -344,6 +335,7 @@ exports[`<FormFactory /> renders correctly 1`] = `
               >
                 <FormControl
                   bsClass="form-control"
+                  className=""
                   componentClass="input"
                   disabled={false}
                   id="required"
@@ -364,13 +356,6 @@ exports[`<FormFactory /> renders correctly 1`] = `
                   />
                 </FormControl>
               </Text>
-              <HelpBlock
-                bsClass="help-block"
-              >
-                <span
-                  className="help-block"
-                />
-              </HelpBlock>
             </div>
           </Col>
         </div>
@@ -634,13 +619,6 @@ exports[`<FormFactory /> renders correctly 1`] = `
                   </div>
                 </ButtonGroup>
               </Dropdown>
-              <HelpBlock
-                bsClass="help-block"
-              >
-                <span
-                  className="help-block"
-                />
-              </HelpBlock>
             </div>
           </Col>
         </div>
@@ -663,6 +641,7 @@ exports[`<FormFactory /> renders correctly 1`] = `
               className="col-sm-5"
             >
               <TextArea
+                className=""
                 disabled={false}
                 id="textarea"
                 key="textarea"
@@ -672,6 +651,7 @@ exports[`<FormFactory /> renders correctly 1`] = `
               >
                 <FormControl
                   bsClass="form-control"
+                  className=""
                   componentClass="textarea"
                   disabled={false}
                   id="textarea"
@@ -689,13 +669,6 @@ exports[`<FormFactory /> renders correctly 1`] = `
                   />
                 </FormControl>
               </TextArea>
-              <HelpBlock
-                bsClass="help-block"
-              >
-                <span
-                  className="help-block"
-                />
-              </HelpBlock>
             </div>
           </Col>
         </div>
@@ -729,6 +702,7 @@ exports[`<FormFactory /> renders correctly 1`] = `
             >
               <Checkbox
                 checked={true}
+                className=""
                 disabled={false}
                 id="checkbox"
                 key="checkbox"
@@ -739,6 +713,7 @@ exports[`<FormFactory /> renders correctly 1`] = `
                 <Checkbox
                   bsClass="checkbox"
                   checked={true}
+                  className=""
                   disabled={false}
                   id="checkbox"
                   inline={false}
@@ -765,13 +740,6 @@ exports[`<FormFactory /> renders correctly 1`] = `
                   </div>
                 </Checkbox>
               </Checkbox>
-              <HelpBlock
-                bsClass="help-block"
-              >
-                <span
-                  className="help-block"
-                />
-              </HelpBlock>
             </div>
           </Col>
         </div>
@@ -816,15 +784,9 @@ exports[`<FormFactory /> renders correctly 1`] = `
               className="col-sm-5"
             >
               <div
+                className=""
                 key="label"
               />
-              <HelpBlock
-                bsClass="help-block"
-              >
-                <span
-                  className="help-block"
-                />
-              </HelpBlock>
             </div>
           </Col>
         </div>
diff --git a/src/components/Wizard/CreateVmWizard/strings.js b/src/components/Wizard/CreateVmWizard/strings.js
index e8d5e0359..36d6bbf00 100644
--- a/src/components/Wizard/CreateVmWizard/strings.js
+++ b/src/components/Wizard/CreateVmWizard/strings.js
@@ -45,7 +45,7 @@ export const ERROR_NO_STORAGE_CLASS_SELECTED = 'Storage Class not selected';
 export const ERROR_NO_STORAGE_SELECTED = 'No storage is selected';
 export const ERROR_STORAGE_NOT_VALID = 'Selected storage is not valid';
 export const ERROR_DISK_NOT_FOUND = 'Disk configuration not found';
-export const HEADER_SIZE = 'Size (GB)';
+export const HEADER_SIZE = 'Size (Gi)';
 export const HEADER_STORAGE_CLASS = 'Storage Class';
 export const REMOVE_DISK_BUTTON = 'Remove Disk';
 export const ATTACH_DISK_BUTTON = 'Attach Disk';
diff --git a/src/components/Wizard/CreateVmWizard/tests/NetworksTab.test.js b/src/components/Wizard/CreateVmWizard/tests/NetworksTab.test.js
index 9336414e6..26debca12 100644
--- a/src/components/Wizard/CreateVmWizard/tests/NetworksTab.test.js
+++ b/src/components/Wizard/CreateVmWizard/tests/NetworksTab.test.js
@@ -168,7 +168,7 @@ describe('<NetworksTab />', () => {
         .find(Dropdown)
         .props().value
     ).toEqual('pxe-net');
-    expect(component.find(HelpBlock).text()).toEqual('');
+    expect(component.find(HelpBlock)).toHaveLength(0);
 
     // add Pod network - not PXE bootable
     component.instance().rowsChanged([podRow], false);
@@ -196,7 +196,7 @@ describe('<NetworksTab />', () => {
         .find(Dropdown)
         .props().value
     ).toEqual('pxe-net');
-    expect(component.find(HelpBlock).text()).toEqual('');
+    expect(component.find(HelpBlock)).toHaveLength(0);
   });
 
   it('does not require bootable network for non-PXE image sources', () => {
diff --git a/src/components/Wizard/CreateVmWizard/tests/__snapshots__/StorageTab.test.js.snap b/src/components/Wizard/CreateVmWizard/tests/__snapshots__/StorageTab.test.js.snap
index 81b47c31e..bcf6cc800 100644
--- a/src/components/Wizard/CreateVmWizard/tests/__snapshots__/StorageTab.test.js.snap
+++ b/src/components/Wizard/CreateVmWizard/tests/__snapshots__/StorageTab.test.js.snap
@@ -37,7 +37,7 @@ exports[`<StorageTab /> renders correctly 1`] = `
         },
         Object {
           "header": Object {
-            "label": "Size (GB)",
+            "label": "Size (Gi)",
             "props": Object {
               "style": Object {
                 "width": "23%",