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

Spinner button #471

Merged
merged 1 commit into from
Dec 21, 2016
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
2 changes: 1 addition & 1 deletion .sass-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ rules:
indentation: 2
leading-zero: 2
nesting-depth: [2, max-depth: 4]
property-sort-order: 2
property-sort-order: 0
property-units: [2, global: ['px', 's']]
quotes: 2
shorthand-values: 2
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
],
"author": "Adslot",
"devDependencies": {
"alexandria-adslot": "^4.0.0",
"alexandria-adslot": "^4.0.2",
"autoprefixer": "^6.5.3",
"babel-core": "^6.18.2",
"babel-eslint": "^6.1.2",
Expand Down
4 changes: 2 additions & 2 deletions scripts/git-hooks/pre-commit
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ then
echo "👾 Installing node modules."
npm install
echo "📑 Compiling distribution files for release."
npm run -s dist && git add dist/*
exit 0
npm run -s dist && git add dist/* && exit 0
exit 1
fi
fi

Expand Down
22 changes: 22 additions & 0 deletions src/components/Main.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import moment from 'moment';
import {
Accordion,
Button,
SpinnerButton,
Checkbox,
ConfirmModal,
DatePicker,
Expand Down Expand Up @@ -327,6 +328,27 @@ class AppComponent extends React.Component {
<div className="btn btn-inverse btn-success">Div</div>
</div>

<h3>Spinner Buttons</h3>
<div className="btn-panel">
<SpinnerButton isLoading className="btn-borderless" bsSize="sm">
Small Borderless
</SpinnerButton>
<SpinnerButton isLoading bsStyle="success" bsSize="large">
Big Success
</SpinnerButton>
<SpinnerButton
isLoading
bsStyle="primary"
>
Primary
</SpinnerButton>
<SpinnerButton disabled>
Disabled
</SpinnerButton>
<SpinnerButton bsStyle="primary">
Not spinning
</SpinnerButton>
</div>

<h1>Tabs</h1>
<div className="btn-panel">
Expand Down
43 changes: 43 additions & 0 deletions src/components/adslotUi/SpinnerButtonComponent.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import _ from 'lodash';
import React, { PropTypes } from 'react';
import classNames from 'classnames';

import Button from 'react-bootstrap/lib/Button';
import { Spinner } from 'alexandria-adslot';
import 'styles/adslotUi/SpinnerButton.scss';
import expandDts from '../../helpers/expandDtsHelper';

const SpinnerButton = (props) => {
const {
isLoading,
children,
dts,
} = props;
Copy link
Contributor

@pjivers pjivers Dec 12, 2016

Choose a reason for hiding this comment

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

Destructure props as arguments...

const SpinnerButton = ({
  isLoading,
  children,
  ...etc...
}) =>

Also, set default value of className (className = '') as

SpinnerButton.defaultProps = {
  className: '',
};

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, nevermind. I didn't see you were using props elsewhere. Instead, just put props argument in parentheses... SpinnerButton = (props) => {

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 couldn't find out how to destructure argument and get the argument at the same time

const SpinnerButton = ({
  isLoading,
  children,
  ...etc...
}) => {
   // I need the props object here
}

Copy link
Contributor

Choose a reason for hiding this comment

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

We generally do destructuring in the arguments of the fn

Copy link
Contributor

Choose a reason for hiding this comment

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

nm, i've seen yours and patricks comments

Copy link
Contributor

@OndrejCholeva OndrejCholeva Dec 14, 2016

Choose a reason for hiding this comment

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

oh, I am sorry to come with late request, but can you add support for passing data-test-selector property down to the button? (https://github.com/Adslot/adslot-ui/blob/8fc657aa6cefd760fecff5f5e063d6f0d1ecaa55/src/components/adslotUi/SplitPaneComponent.jsx as inspiration)

  loadingHoverElement = children,
  dts #yeah, I know it is a shortcut, but that's what we use...
} = props;

...
<Button
     {..._.pick(props, _.keys(Button.propTypes))}
     expandDts(dts)

...

   dts: PropTypes.string

pseudocode


return (
<Button
disabled={isLoading || props.disabled}
{..._.pick(props, _.keys(Button.propTypes))}
className={classNames('spinner-button-component', props.className)}
{...expandDts(dts)}
>
{isLoading ?
<div className="spinner-container">
<Spinner size={_.includes(['lg', 'large'], props.bsSize) ? 'medium' : 'small'} />
</div>
: null }
<div className={isLoading ? 'spinner-button-component-children-container' : null}>{children}</div>
</Button>
);
};

SpinnerButton.propTypes = _.assign({
isLoading: PropTypes.bool,
dts: PropTypes.string,
}, Button.propTypes);

SpinnerButton.defaultProps = {
isLoading: false,
};

export default SpinnerButton;
18 changes: 10 additions & 8 deletions src/components/distributionEntry.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// Export the consumable components.
import Accordion from 'components/adslotUi/AccordionComponent';
import Button from 'react-bootstrap/lib/Button';
import SearchBar from 'components/adslotUi/SearchBarComponent';
import Checkbox from 'react-icheck/lib/Checkbox';
import ConfirmModal from 'components/adslotUi/ConfirmModalComponent';
Expand All @@ -23,6 +22,8 @@ import SplitPane from 'components/adslotUi/SplitPaneComponent';
import Tab from 'react-bootstrap/lib/Tab';
import Tabs from 'react-bootstrap/lib/Tabs';
import Toggle from 'react-toggle';
import SpinnerButton from 'components/adslotUi/SpinnerButtonComponent';
import Button from 'react-bootstrap/lib/Button';
import TreePickerGrid from 'components/adslotUi/TreePickerGridComponent';
import TreePickerNav from 'components/adslotUi/TreePickerNavComponent';
import TreePickerNode from 'components/adslotUi/TreePickerNodeComponent';
Expand Down Expand Up @@ -50,19 +51,20 @@ import {
Totals,
} from 'alexandria-adslot';

require('styles/_bootstrap-custom.scss');
require('styles/_icheck-custom.scss');
require('styles/_react-select-custom.scss');
require('styles/_react-toggle-custom.scss');
require('styles/_react-datepicker-custom.scss');
import 'styles/_bootstrap-custom.scss';
import 'styles/_icheck-custom.scss';
import 'styles/_react-select-custom.scss';
import 'styles/_react-toggle-custom.scss';
import 'styles/_react-datepicker-custom.scss';
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe, SCSS files still need to be required so that webpack creates a separate css file.

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 tried dist in both cases and saw only one css ouput, had content of those files. Am I missing something?

Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome! Just trash your dist file and rebuild; double-check that it's still all good :)


module.exports = {
export {
Accordion,
Alert,
Avatar,
BorderedWell,
Breadcrumb,
Button,
SpinnerButton,
Card,
Checkbox,
ConfirmModal,
Expand All @@ -88,7 +90,7 @@ module.exports = {
Radio,
RadioGroup,
SearchBar,
SearchField: Search,
Search as SearchField,
Select,
Slicey,
SplitPane,
Expand Down
2 changes: 1 addition & 1 deletion src/examples/exampleEntry.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import ExampleForm from './components/forms';
import ExampleSelect from './components/selects';

module.exports = {
export {
ExampleForm,
ExampleSelect,
};
26 changes: 26 additions & 0 deletions src/styles/adslotUi/SpinnerButton.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
.spinner-button-component {
&-children-container {
visibility: hidden; // preserves width and height of button
}

.spinner-container {
position: absolute;
right: 0;
left: 0;
margin-right: auto;
margin-left: auto;

// workaround for vertical centering
.spinner-component {
.spinner {
&-small {
margin-top: 1px;
}

&-medium {
margin-top: -3px;
}
}
}
}
}
45 changes: 45 additions & 0 deletions test/components/adslotUi/SpinnerButtonComponentTest.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import React from 'react';
import SpinnerButton from 'components/adslotUi/SpinnerButtonComponent';
import { Spinner } from 'alexandria-adslot';
import { shallow } from 'enzyme';

describe('SpinnerButtonComponent', () => {
it('should render with defaults', () => {
const element = shallow(<SpinnerButton>Test</SpinnerButton>);
expect(element.find(Spinner)).to.have.length(0);
const buttonElement = element.find('Button');
expect(buttonElement.prop('disabled')).to.equal(false);
expect(buttonElement.prop('isLoading')).to.equal(undefined);
expect(element.children().last().text()).to.equal('Test');
});

it('should pass props to button', () => {
const element = shallow(
<SpinnerButton dts="test" bsStyle="primary" bsSize="lg" isLoading>Test</SpinnerButton>
);
expect(element.find(Spinner)).to.have.length(1);

const buttonElement = element.find('Button[data-test-selector="test"]');
expect(buttonElement.prop('bsStyle')).to.equal('primary');
expect(buttonElement.prop('bsSize')).to.equal('lg');
expect(buttonElement.prop('isLoading')).to.equal(undefined);
});

it('should be disabled in loading mode', () => {
const element = shallow(
<SpinnerButton isLoading>Test</SpinnerButton>
);
const buttonElement = element.find('Button');
expect(buttonElement.prop('disabled')).to.equal(true);
});

it('should preserve disabled state even when not loading', () => {
const element = shallow(
<SpinnerButton disabled>Test</SpinnerButton>
);

const buttonElement = element.find('Button');
expect(buttonElement.prop('isLoading')).to.equal(undefined);
expect(buttonElement.prop('disabled')).to.equal(true);
});
});