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 all 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
3 changes: 3 additions & 0 deletions app/ButtonsWrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { BaseButton, Button, IconButton, IconsButton } from "../composites/Plugi
import IconButtonToggle from "../composites/Plugin/Shared/components/IconButtonToggle";
import { BaseLinkButton, LinkButton } from "../composites/Plugin/Shared/components/LinkButton";
import FormButton from "../forms/Button";
import Toggle from "../composites/Plugin/Shared/components/Toggle.js"
import IconLabelledButton from "../composites/Plugin/Shared/components/IconLabelledButton";

const ButtonsContainer = styled.div`
Expand Down Expand Up @@ -88,6 +89,8 @@ export default class ButtonsList extends React.Component {
<Separator />
<YoastButton>YoastButton</YoastButton>
<Separator />
<Toggle ariaLabel="Test the Toggle"/>
<Separator />
<IconLabelledButton icon="question-circle">Need help?</IconLabelledButton>
<IconLabelledButton icon="gear">Settings</IconLabelledButton>
<IconLabelledButton
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import React from "react";
import PropTypes from "prop-types";
import styled from "styled-components";
import Toggle from "../../Shared/components/Toggle";
import { __ } from "@wordpress/i18n";

const Cornerstone = styled.div`
display: flex;
justify-content: space-between;
align-items: center;
margin-top: 5px;

label {
margin-right: 10px;
flex-shrink: 0;
max-width: 75%;
}
`;

class CornerstoneToggle extends React.Component {
/**
* Renders the CornerstoneToggle component.
*
* @returns {ReactElement} the CornerstoneToggle component.
*/
render() {
return (
<Cornerstone>
<Toggle
id="cornerstone_toggle"
ariaLabel={ __( "Mark this post as cornerstone content", "yoast-components" ) }
labelText={ __( "Mark this as cornerstone content.", "yoast-components" ) }
isEnabled={ this.props.isEnabled }
onSetToggleState={ this.props.onToggle }
onToggleDisabled={ this.props.onToggleDisabled }
/>
</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.

isEnabled: PropTypes.bool,
onSetToggleState: PropTypes.func,
onToggle: PropTypes.func,
disable: PropTypes.bool,
onToggleDisabled: PropTypes.func,
};

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,112 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`The CornerstoneToggle matches the snapshot 1`] = `
.c2 {
background-color: #ccc;
border-radius: 7px;
height: 14px;
width: 30px;
cursor: pointer;
margin: 0;
outline: 0;
}

.c2:focus > span {
box-shadow: inset 0 0 0 1px #fff,0 0 0 1px #5b9dd9,0 0 2px 1px rgba(30,140,190,.8);
}

.c3 {
background-color: #888;
margin-left: -2px;
box-shadow: 0 2px 2px 2px rgba(0,0,0,0.1);
border-radius: 100%;
height: 20px;
width: 20px;
position: absolute;
margin-top: -3px;
}

.c4 {
font-size: 14px;
line-height: 20px;
width: 30px;
text-align: center;
display: inline-block;
margin: 0;
font-style: italic;
}

.c1 {
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 {
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;
margin-top: 5px;
}

.c0 label {
margin-right: 10px;
-webkit-flex-shrink: 0;
-ms-flex-negative: 0;
flex-shrink: 0;
max-width: 75%;
}

<div
className="c0"
>
<div
className="c1"
>
<label
htmlFor="cornerstone_toggle"
onClick={[Function]}
>
Mark this as cornerstone content.
</label>
<div
aria-checked={false}
aria-label="Mark this post as cornerstone content"
className="c2"
id="cornerstone_toggle"
onClick={[Function]}
onKeyDown={[Function]}
role="checkbox"
tabIndex="0"
>
<span
className="c3"
/>
</div>
<span
aria-hidden="true"
className="c4"
>
Off
</span>
</div>
</div>
`;
8 changes: 4 additions & 4 deletions composites/Plugin/Shared/components/Button.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ export const addButtonStyles = flow( [
*
* @param {object} props Component props.
*
* @returns {ReactElement} Styled button.
* @returns {ReactElement} styled button.
*/
export const BaseButton = addButtonStyles(
styled.button`
Expand Down Expand Up @@ -203,7 +203,7 @@ BaseButton.defaultProps = {
*
* @param {object} props Component props.
*
* @returns {ReactElement} Styled button.
* @returns {ReactElement} styled button.
*/
export const Button = addFontSizeStyles( BaseButton );

Expand All @@ -226,7 +226,7 @@ function addIconTextStyle( icon ) {
*
* @param {object} props Component props.
*
* @returns {ReactElement} Styled icon button.
* @returns {ReactElement} styled icon button.
*/
export const IconButton = ( props ) => {
const { children: text, icon, iconColor } = props;
Expand Down Expand Up @@ -265,7 +265,7 @@ IconButton.defaultProps = {
*
* @param {object} props Component props.
*
* @returns {ReactElement} Styled icon button.
* @returns {ReactElement} styled icon button.
*/
export const IconsButton = ( props ) => {
const { children: text, prefixIcon, prefixIconColor, suffixIcon, suffixIconColor } = props;
Expand Down
2 changes: 1 addition & 1 deletion composites/Plugin/Shared/components/HelpCenterButton.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import SvgIcon from "../../Shared/components/SvgIcon";
*
* @param {object} props Component props.
*
* @returns {ReactElement} Styled icon button.
* @returns {ReactElement} styled icon button.
*/
const HelpCenterButtonBase = ( props ) => {
return (
Expand Down
4 changes: 2 additions & 2 deletions composites/Plugin/Shared/components/LinkButton.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { addButtonStyles, addFontSizeStyles } from "./Button";
*
* @param {object} props Component props.
*
* @returns {ReactElement} Styled button.
* @returns {ReactElement} styled button.
*/
export const BaseLinkButton = addButtonStyles(
styled.a`
Expand Down Expand Up @@ -59,6 +59,6 @@ BaseLinkButton.defaultProps = {
*
* @param {object} props Component props.
*
* @returns {ReactElement} Styled link.
* @returns {ReactElement} styled link.
*/
export const LinkButton = addFontSizeStyles( BaseLinkButton );
135 changes: 135 additions & 0 deletions composites/Plugin/Shared/components/Toggle.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
import PropTypes from "prop-types";
import React from "react";
import styled from "styled-components";
import colors from "../../../../style-guide/colors.json";
import { __ } from "@wordpress/i18n";

const ToggleBar = styled.div`
background-color: ${ props => props.isEnabled ? "#a5d6a7" : colors.$color_button_border };
border-radius: 7px;
height: 14px;
width: 30px;
cursor: pointer;
margin: 0;
outline: 0;
&:focus > span {
box-shadow: inset 0 0 0 1px ${colors.$color_white}, 0 0 0 1px #5b9dd9, 0 0 2px 1px rgba(30, 140, 190, .8);
Copy link
Contributor

Choose a reason for hiding this comment

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

Unsure as to why the first color is variable, but the other two are not.

Copy link
Contributor

Choose a reason for hiding this comment

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

This applies also to "#a5d6a7" on line 8. When new colors are needed, please discuss with the design team: "our" colors should all be in the color palette (_colors.scss). Of course this makes sense when a new color is going to be used extensively. For colors that are used only once, we should wonder if they're really needed in the first place. Please consider this is essential to be able to keep our color palette under control in the long run. /Cc @hedgefield

In a way, #5b9dd9 and rgba(30, 140, 190, .8) are an exception, as they're the colors WordPress uses for the focus style. Normally, they're inherited on interactive elements. If we're going to use them extensively on non-natively interactive elements then we may want to consider to add them to the color palette. Worth noting Gutenberg uses slightly different colors 😬

}
`;

const ToggleBullet = styled.span`
background-color: ${ props => props.isEnabled ? colors.$color_green_medium_light : colors.$color_grey_medium_dark };
margin-left: ${ props => props.isEnabled ? "12px" : "-2px" };
box-shadow: 0 2px 2px 2px rgba(0, 0, 0, 0.1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here: this is a hardcoded color. We should use a color from the palette (not necessarily a rgba color)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi Andrea, you are right. But we'll skip this for now to finish the component. We'll pick this up again during the polishing process.

border-radius: 100%;
height: 20px;
width: 20px;
position: absolute;
margin-top: -3px;
`;

const ToggleVisualLabel = styled.span`
font-size: 14px;
line-height: 20px;
width: 30px;
text-align: center;
display: inline-block;
margin: 0;
font-style: italic;
`;

const ToggleDiv = styled.div`
display: flex;
justify-content: space-between;
align-items: center;
`;

class Toggle extends React.Component {
/**
* Sets the toggle object.
*
* @param {Object} props The props to use.
*
* @returns {void}
*/
constructor( props ) {
super( props );

this.onClick = this.props.onToggleDisabled;

this.setToggleState = this.setToggleState.bind( this );

if ( props.disable !== true ) {
this.onClick = this.setToggleState.bind( this );
}
}

/**
* Returns the rendered HTML.
*
* @returns {ReactElement} The rendered HTML.
*/
render() {
return(
<ToggleDiv>
{ /* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-noninteractive-element-interactions */ }
<label
htmlFor={ this.props.id }
Copy link
Contributor

@boblinthorst boblinthorst Jul 30, 2018

Choose a reason for hiding this comment

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

Not important enough for now, and also more a preference than an instruction.

but when props are used in multiple places, it's nice to extract props to local variables, this is done, for example, in SnippetEditorFields.js.

onClick={ this.onClick }
>
{ this.props.labelText }
</label>
<ToggleBar
isEnabled={ this.props.isEnabled }
onClick={ this.onClick }
onKeyDown={ this.setToggleState }
tabIndex="0"
role="checkbox"
aria-label={ this.props.ariaLabel }
aria-checked={ this.props.isEnabled }
id={ this.props.id }
>
<ToggleBullet isEnabled={ this.props.isEnabled } />
</ToggleBar>
<ToggleVisualLabel aria-hidden="true">
{ this.props.isEnabled ? __( "On", "yoast-components" ) : __( "Off", "yoast-components" ) }
</ToggleVisualLabel>
</ToggleDiv>
);
}

/**
* Sets the state to the opposite of the current state.
*
* @param {Object} event React SyntheticEvent.
*
* @returns {void}
*/
setToggleState( event ) {
// Makes the toggle actionable with the Space bar key.
if ( event.type === "keydown" && event.keyCode !== 32 ) {
return;
}

this.props.onSetToggleState( ! this.props.isEnabled );
}
}

Toggle.propTypes = {
isEnabled: PropTypes.bool,
ariaLabel: PropTypes.string.isRequired,
onSetToggleState: PropTypes.func,
disable: PropTypes.bool,
onToggleDisabled: PropTypes.func,
id: PropTypes.string.isRequired,
labelText: PropTypes.string,
};

Toggle.defaultProps = {
isEnabled: false,
onSetToggleState: () => {},
disable: false,
labelText: "",
};

export default Toggle;
Loading