-
Notifications
You must be signed in to change notification settings - Fork 6
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
10267 add cornerstone toggle to sidebar #668
Changes from 5 commits
cbe54a0
21dcb54
5b583cf
574b38b
bec7af7
8a15cf0
3e2531a
46bf775
a332e47
2500e78
2c231d6
b0eadd2
5174081
442ff3b
42110ff
f243ce6
5dc58b2
dbfce44
bb476f8
50bd497
a546717
43a0d17
151386e
7a43c0e
d626b5a
e53ea6a
a17346c
224c201
8b863c6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
import React from "react"; | ||
import PropTypes from "prop-types"; | ||
import Styled from "styled-components"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Styled should be lowercase |
||
import noop from "lodash/noop"; | ||
|
||
import GutenbergToggle from "../../Shared/components/GutenbergToggle"; | ||
|
||
const Cornerstone = Styled.div` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Styled should be lowercase |
||
display: flex; | ||
justify-content: space-between; | ||
align-items: center; | ||
|
||
label { | ||
margin-right: 10px; | ||
flex-shrink: 0; | ||
max-width: 75%; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the indentation in all these lines is made of spaces. Should be tabs. |
||
`; | ||
|
||
class CornerstoneToggle extends React.Component { | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. eslint "warning Block must not be padded by blank lines", please remove this blank line |
||
/** | ||
* The constructor. | ||
* | ||
* @param {object} props The component props. | ||
* | ||
* @returns {void} | ||
*/ | ||
constructor( props ) { | ||
super( props ); | ||
|
||
this.state = { | ||
cornerstoneToggleState: this.props.checked, | ||
}; | ||
|
||
this.handleChange = this.handleChange.bind( this ); | ||
} | ||
|
||
/** | ||
* Handles changes on the cornerstoneToggle. | ||
* | ||
* @returns {void} | ||
*/ | ||
handleChange() { | ||
this.setState( { cornerstoneToggleState: ! this.state.cornerstoneToggleState } ); | ||
this.props.onChange(); | ||
} | ||
|
||
/** | ||
* Renders the CornerstoneToggle component. | ||
* | ||
* @returns {ReactElement} the CornerstoneToggle component. | ||
*/ | ||
render() { | ||
return ( | ||
<Cornerstone> | ||
<label htmlFor={this.props.key}>Cornerstone</label> | ||
<GutenbergToggle | ||
key={this.props.key} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not sure why there's the need of a key? |
||
checked={this.state.cornerstoneToggleState} | ||
onChange={this.handleChange} | ||
id={this.props.toggleId} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. coding standards: all these lines (57 and 59-62) miss spaces between brackets. The label should be always correctly associated with the input ID, more details in the CR comment. |
||
/> | ||
</Cornerstone> | ||
); | ||
} | ||
} | ||
|
||
CornerstoneToggle.propTypes = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We usually set defaults for PropTypes that are not required. |
||
key: PropTypes.string, | ||
checked: PropTypes.bool, | ||
onChange: PropTypes.func.isRequired, | ||
toggleId: PropTypes.string, | ||
}; | ||
|
||
CornerstoneToggle.defaultProps = { | ||
checked: false, | ||
onChange: noop, | ||
}; | ||
|
||
export default CornerstoneToggle; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
import React from "react"; | ||
import renderer from "react-test-renderer"; | ||
import CornerstoneToggle from "../components/CornerstoneToggle"; | ||
|
||
test( "The CornerstoneToggle matches the snapshot", () => { | ||
const component = renderer.create( | ||
<CornerstoneToggle onChange={ () => {} } checked={ true } /> | ||
); | ||
|
||
let tree = component.toJSON(); | ||
expect( tree ).toMatchSnapshot(); | ||
} ); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
// Jest Snapshot v1, https://goo.gl/fbAQLP | ||
|
||
exports[`The CornerstoneToggle matches the snapshot 1`] = ` | ||
.c0 { | ||
display: -webkit-box; | ||
display: -webkit-flex; | ||
display: -ms-flexbox; | ||
display: flex; | ||
-webkit-box-pack: justify; | ||
-webkit-justify-content: space-between; | ||
-ms-flex-pack: justify; | ||
justify-content: space-between; | ||
-webkit-align-items: center; | ||
-webkit-box-align: center; | ||
-ms-flex-align: center; | ||
align-items: center; | ||
} | ||
|
||
.c0 label { | ||
margin-right: 10px; | ||
-webkit-flex-shrink: 0; | ||
-ms-flex-negative: 0; | ||
flex-shrink: 0; | ||
max-width: 75%; | ||
} | ||
|
||
<div | ||
className="c0" | ||
> | ||
<label | ||
htmlFor={undefined} | ||
> | ||
Cornerstone | ||
</label> | ||
<span | ||
className="components-form-toggle is-checked" | ||
> | ||
<input | ||
checked={true} | ||
className="components-form-toggle__input" | ||
id={undefined} | ||
onChange={[Function]} | ||
type="checkbox" | ||
/> | ||
<span | ||
className="components-form-toggle__track" | ||
/> | ||
<span | ||
className="components-form-toggle__thumb" | ||
/> | ||
<svg | ||
aria-hidden="true" | ||
className="components-form-toggle__on" | ||
focusable="false" | ||
height="6" | ||
role="img" | ||
viewBox="0 0 2 6" | ||
width="2" | ||
xmlns="http://www.w3.org/2000/svg" | ||
> | ||
<path | ||
d="M0 0h2v6H0z" | ||
/> | ||
</svg> | ||
</span> | ||
</div> | ||
`; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import React from "react"; | ||
import classnames from "classnames"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. has the introduction of the classnames package been discussed with @atimmer ? |
||
import noop from "lodash/noop"; | ||
|
||
function GutenbergToggle( { className, checked, id, onChange = noop, ...props } ) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. eslint error Missing JSDoc comment for this function |
||
const wrapperClasses = classnames( | ||
'components-form-toggle', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. eslint error Strings must use doublequote |
||
className, | ||
{ 'is-checked': checked }, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. eslint error Strings must use doublequote |
||
); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so to my understanding, this gets properly styled only when in Gutenberg. When used outside Gutenberg, it will be completely unstyled (as in the standalone app example > "Keyword" tab). Is this what we want? |
||
return ( | ||
<span className={ wrapperClasses }> | ||
<input | ||
className="components-form-toggle__input" | ||
id={ id } | ||
type="checkbox" | ||
checked={ checked } | ||
onChange={ onChange } | ||
{ ...props } | ||
/> | ||
<span className="components-form-toggle__track"></span> | ||
<span className="components-form-toggle__thumb"></span> | ||
{ checked ? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. eslint error '?' should be placed at the beginning of the line |
||
<svg className="components-form-toggle__on" width="2" height="6" aria-hidden="true" role="img" focusable="false" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 2 6"><path d="M0 0h2v6H0z" /></svg> : <svg className="components-form-toggle__off" width="6" height="6" aria-hidden="true" role="img" focusable="false" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 6 6"><path d="M3 1.5c.8 0 1.5.7 1.5 1.5S3.8 4.5 3 4.5 1.5 3.8 1.5 3 2.2 1.5 3 1.5M3 0C1.3 0 0 1.3 0 3s1.3 3 3 3 3-1.3 3-3-1.3-3-3-3z" /></svg> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. eslint error Line 28 exceeds the maximum line length of 150 |
||
} | ||
</span> | ||
); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please check indentation from line 24 to line 31 |
||
} | ||
|
||
export default GutenbergToggle; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this path should be "../composites/Plugin/CornerstoneContent/components/CornerstoneToggle"