Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #30638 - replace react-numeric-input with rc-number-input #7896

Merged
merged 1 commit into from
Sep 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .stylelintrc
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
"stylelint-config-standard",
],
"rules": {
"selector-type-no-unknown": null
"at-rule-no-unknown": null,
"selector-type-no-unknown": null,
},
"ignore": ["custom-elements"],
}
12 changes: 6 additions & 6 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,18 @@
"create-react-component": "yo react-domain"
},
"dependencies": {
"@theforeman/vendor": "^4.12.0",
"@theforeman/vendor": "^4.14.0",
"intl": "~1.2.5",
"jed": "^1.1.1",
"react-intl": "^2.8.0"
},
"devDependencies": {
"@babel/core": "^7.7.0",
"@theforeman/builder": "^4.12.0",
"@theforeman/eslint-plugin-foreman": "^4.12.0",
"@theforeman/stories": "^4.12.0",
"@theforeman/test": "^4.12.0",
"@theforeman/vendor-dev": "^4.12.0",
"@theforeman/builder": "^4.14.0",
"@theforeman/eslint-plugin-foreman": "^4.14.0",
"@theforeman/stories": "^4.14.0",
"@theforeman/test": "^4.14.0",
"@theforeman/vendor-dev": "^4.14.0",
"argv-parse": "^1.0.1",
"babel-eslint": "^10.0.0",
"babel-loader": "^8.0.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,14 @@ exports[`CPUCoresInput rendering should render with default props 1`] = `
>
<NumericInput
className=""
disabled={false}
format={null}
label="CPUs"
minValue={1}
onChange={[Function]}
precision={0}
readOnly={false}
step={1}
value={1}
/>
</Col>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,3 @@
.help-block-icon {
margin-right: 3px;
}

.common-numericInput {
.react-numeric-input {
display: flex !important;

input {
flex: 1;
height: 28px;
}
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import ReactNumericInput from 'react-numeric-input';
import RCInputNumber from 'rc-input-number';
import React from 'react';
import PropTypes from 'prop-types';
import './NumericInput.scss';

import { noop } from '../../../common/helpers';
import CommonForm from './CommonForm';
Expand All @@ -11,16 +12,25 @@ const NumericInput = ({
value,
onChange,
format,
parser,
step,
precision,
minValue,
disabled,
readOnly,
}) => (
<CommonForm label={label} className={`common-numericInput ${className}`}>
<ReactNumericInput
format={format}
<CommonForm label={label} className={className}>
<RCInputNumber
formatter={format}
parser={parser}
step={step}
min={minValue}
value={value}
precision={precision}
onChange={onChange}
disabled={disabled}
readOnly={readOnly}
prefixCls="foreman-numeric-input"
/>
</CommonForm>
);
Expand All @@ -30,19 +40,27 @@ NumericInput.propTypes = {
className: PropTypes.string,
value: PropTypes.oneOfType([PropTypes.number, PropTypes.string]),
format: PropTypes.func,
parser: PropTypes.func,
step: PropTypes.number,
precision: PropTypes.number,
minValue: PropTypes.number,
onChange: PropTypes.func,
readOnly: PropTypes.bool,
disabled: PropTypes.bool,
};

NumericInput.defaultProps = {
label: '',
className: '',
value: 0,
format: null,
parser: undefined,
step: 1,
precision: 0,
minValue: 0,
onChange: noop,
disabled: false,
readOnly: false,
};

export default NumericInput;
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
@import "~@theforeman/vendor/scss/variables";
@import "~@theforeman/vendor/scss/mixins";

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You add @import "~@theforeman/vendor/scss/variables"; to the beginning of the file to have access to pf3/pf4 scss variables that you can use instead hard-coded values.

IMO it makes sense to reuse them for colors, shadows, sizes, and spaces.

Those are all the variables and the mixins available for use:

It might be easier to search using the patternfly docs:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this import and change the colors and some of the sizes. Let me know if I missed some changes.

.foreman-numeric-input {
position: relative;
display: flex;

.foreman-numeric-input-input-wrap {
width: 100%;

// Remove input style when this PR is merged: https://github.com/react-component/input-number/pull/262
input {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could drop the input styles if we could set the input className to form-control.
Unfortunately, the rc-input-number doesn't support custom class names on the input field.

Sent a PR to rc-input-number so they can have this feature:
react-component/input-number#262

Can you please add a comment in the code with a reminder to drop the input styles once merged?

flex: 1;
height: 26px;
width: 100%;
padding-right: $padding-small-vertical;
box-sizing: border-box;
font-size: inherit;
border: 1px solid $input-border;
border-radius: $input-border-radius;
padding-left: $padding-small-vertical;
display: block;
appearance: none;
line-height: normal;

&:focus {
@include form-control-outline;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected unknown at-rule "@include" at-rule-no-unknown

}

&:hover {
border-color: $input-border-hover;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I set the disabled prop to true, the hover effect still exists and it looks odd.

}

&[disabled] {
box-shadow: none;
border-color: $input-border;
color: #8b8d8f;
cursor: not-allowed;
}
}
}

.foreman-numeric-input-handler-up-inner,
.foreman-numeric-input-handler-down-inner {
position: absolute;
top: 50%;
left: 50%;
border-style: solid;
margin: -0.3ex 0 0 -0.56ex;
}

.foreman-numeric-input-handler-up-inner {
border-width: 0 0.6ex 0.6ex 0.6ex;
border-color: transparent transparent $color-pf-black-600;
}

.foreman-numeric-input-handler-down-inner {
border-width: 0.6ex 0.6ex 0 0.6ex;
border-color: $color-pf-black-600 transparent transparent;
}

.foreman-numeric-input-handler-up {
top: 2px;
bottom: 50%;
border-radius: 2px 2px 0 0;
border-width: 1px 1px 0;
}

.foreman-numeric-input-handler-down {
top: 50%;
bottom: 2px;
border-radius: 0 0 2px 2px;
border-width: 0 1px 1px 1px;
}

.foreman-numeric-input-handler-up,
.foreman-numeric-input-handler-down {
position: absolute;
right: 2px;
width: 2.26ex;
border-color: $color-pf-black-300;
border-style: solid;
text-align: center;
cursor: default;
transition: all 0.1s ease 0s;
background: $color-pf-black-300;
box-shadow: $color-pf-black-300 -1px -1px 3px inset, $color-pf-black-200 1px 1px 3px inset;

&:hover {
background: $color-pf-black-400;
}

&:active {
background: $color-pf-black-500;
box-shadow: 0 1px 3px $color-pf-black-300 inset, -1px -1px 4px $color-pf-black-200 inset;
}

&:disabled {
opacity: 0.5;
box-shadow: none;
cursor: not-allowed;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,5 @@
.common-checkbox {
text-align: left;
}

.common-numericInput {
.react-numeric-input {
display: flex !important;

input {
flex: 1;
height: 28px;
}
}
}
}
}