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

10267 add cornerstone toggle to sidebar #668

Merged
merged 29 commits into from
Jul 30, 2018
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
cbe54a0
temporary commit
Dieterrr Jul 17, 2018
21dcb54
Added Gutenberg toggle and Cornerstone toggle
Dieterrr Jul 17, 2018
5b583cf
Added state manangement for now
boblinthorst Jul 19, 2018
574b38b
Added tests
Dieterrr Jul 19, 2018
bec7af7
Removed one test
Dieterrr Jul 19, 2018
8a15cf0
Fixed CS errors
Dieterrr Jul 19, 2018
3e2531a
Updated lodash, added components, added babel runtime, fixed CS
Dieterrr Jul 23, 2018
46bf775
updated package.json
Dieterrr Jul 23, 2018
a332e47
Fixed CS
Dieterrr Jul 23, 2018
2500e78
reverted to own component
Dieterrr Jul 25, 2018
2c231d6
reverted package.json
Dieterrr Jul 25, 2018
b0eadd2
Gave toggle a default id to associate with label
Dieterrr Jul 25, 2018
5174081
Added textual help to clarify toggle position
Dieterrr Jul 25, 2018
442ff3b
added snapshot
Dieterrr Jul 25, 2018
42110ff
Fixed CS
Dieterrr Jul 25, 2018
f243ce6
Added Toggle from myYoast, replaced Gutenbergtoggle, added it to app.js
Dieterrr Jul 26, 2018
5dc58b2
Made MyYoast toggle clickable by label, added prop for custom label
Dieterrr Jul 26, 2018
dbfce44
Fixed CS errors
Dieterrr Jul 26, 2018
bb476f8
classnames dependency not needed
maartenleenders Jul 26, 2018
50bd497
Fixed ESlint error, refactored to make code prettier. Added export of…
Dieterrr Jul 26, 2018
a546717
reverted changes
Dieterrr Jul 26, 2018
43a0d17
Merged with trunk, added default props
Dieterrr Jul 26, 2018
151386e
Implemented changes bases on CR
Dieterrr Jul 27, 2018
7a43c0e
Refactored and added export
Dieterrr Jul 27, 2018
d626b5a
Updated snap
Dieterrr Jul 27, 2018
e53ea6a
Improved toggle code
Jul 30, 2018
a17346c
Merge branch '10267-add-cornerstone-toggle-to-sidebar' of https://git…
Jul 30, 2018
224c201
Added missing props validation
Jul 30, 2018
8b863c6
Merged with develop
Dieterrr Jul 30, 2018
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: 2 additions & 0 deletions app/KeywordExample.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import styled from "styled-components";
// Internal dependencies.
import KeywordInput from "../composites/Plugin/Shared/components/KeywordInput";
import SynonymsSection from "../composites/Plugin/Synonyms/components/SynonymsSection";
import CornerstoneToggle from "../composites/Plugin/CornerstoneContent/CornerstoneToggle";
Copy link
Contributor

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"


const Container = styled.div`
background-color: white;
Expand Down Expand Up @@ -32,6 +33,7 @@ export default class KeywordExample extends Component {
label={ "Keyword synonyms" }
onChange={ synonyms => console.log( 'SynonymsField change event', synonyms ) }
/>
<CornerstoneToggle />
</Container>
);
}
Expand Down
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";
Copy link
Contributor

Choose a reason for hiding this comment

The 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`
Copy link
Contributor

Choose a reason for hiding this comment

The 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%;
}
Copy link
Contributor

Choose a reason for hiding this comment

The 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 {

Copy link
Contributor

Choose a reason for hiding this comment

The 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}
Copy link
Contributor

Choose a reason for hiding this comment

The 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}
Copy link
Contributor

Choose a reason for hiding this comment

The 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 = {
Copy link
Contributor

@maartenleenders maartenleenders Jul 26, 2018

Choose a reason for hiding this comment

The 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>
`;
34 changes: 34 additions & 0 deletions composites/Plugin/Shared/components/GutenbergToggle.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/**
* External dependencies
*/
import React from "react";
import classnames from "classnames";
Copy link
Contributor

Choose a reason for hiding this comment

The 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 } ) {
Copy link
Contributor

@afercia afercia Jul 19, 2018

Choose a reason for hiding this comment

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

eslint error Missing JSDoc comment for this function
eslint error 'className' is missing in props validation
eslint error 'checked' is missing in props validation
eslint error 'id' is missing in props validation
eslint error 'onChange' is missing in props validation

const wrapperClasses = classnames(
'components-form-toggle',
Copy link
Contributor

@afercia afercia Jul 19, 2018

Choose a reason for hiding this comment

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

eslint error Strings must use doublequote

className,
{ 'is-checked': checked },
Copy link
Contributor

Choose a reason for hiding this comment

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

eslint error Strings must use doublequote

);

Copy link
Contributor

Choose a reason for hiding this comment

The 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 ?
Copy link
Contributor

Choose a reason for hiding this comment

The 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>
Copy link
Contributor

Choose a reason for hiding this comment

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

eslint error Line 28 exceeds the maximum line length of 150

}
</span>
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please check indentation from line 24 to line 31

}

export default GutenbergToggle;
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
"@wordpress/a11y": "^1.0.7",
"@wordpress/i18n": "^1.1.0",
"algoliasearch": "^3.22.3",
"classnames": "^2.2.6",
"debug": "^3.0.0",
"draft-js": "^0.10.5",
"draft-js-mention-plugin": "^3.0.4",
Expand Down