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.

Sign up for GitHub

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

Try polishing/simpilfying the header area #5300

Merged
merged 7 commits into from
Feb 28, 2018
Merged

Try polishing/simpilfying the header area #5300

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
68c4b22
Change Preview iconbutton to a button.
Feb 28, 2018
b0d72a0
Add new icons, and change "Save Draft" and Switch to Draft buttons
Feb 28, 2018
5984928
Add back Shortcode icon.
Feb 28, 2018
4ebd646
Fix some mobile issues
Feb 28, 2018
2a69d5e
Tests: Update header Save button tests
aduth Feb 28, 2018
17e5c94
Address feedback
Feb 28, 2018
8a1b678
Further iconbutton simplification.
Feb 28, 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
8 changes: 7 additions & 1 deletion components/dashicon/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,14 @@ export default class Dashicon extends wp.element.Component {
case 'clock':
path = 'M10 2c4.42 0 8 3.58 8 8s-3.58 8-8 8-8-3.58-8-8 3.58-8 8-8zm0 14c3.31 0 6-2.69 6-6s-2.69-6-6-6-6 2.69-6 6 2.69 6 6 6zm-.71-5.29c.07.05.14.1.23.15l-.02.02L14 13l-3.03-3.19L10 5l-.97 4.81h.01c0 .02-.01.05-.02.09S9 9.97 9 10c0 .28.1.52.29.71z';
break;
case 'cloud-saved':
path = 'M14.8 9c.1-.3.2-.6.2-1 0-2.2-1.8-4-4-4-1.5 0-2.9.9-3.5 2.2-.3-.1-.7-.2-1-.2C5.1 6 4 7.1 4 8.5c0 .2 0 .4.1.5-1.8.3-3.1 1.7-3.1 3.5C1 14.4 2.6 16 4.5 16h10c1.9 0 3.5-1.6 3.5-3.5 0-1.8-1.4-3.3-3.2-3.5zm-6.3 5.9l-3.2-3.2 1.4-1.4 1.8 1.8 3.8-3.8 1.4 1.4-5.2 5.2z';
break;
case 'cloud-upload':
path = 'M14.8 9c.1-.3.2-.6.2-1 0-2.2-1.8-4-4-4-1.5 0-2.9.9-3.5 2.2-.3-.1-.7-.2-1-.2C5.1 6 4 7.1 4 8.5c0 .2 0 .4.1.5-1.8.3-3.1 1.7-3.1 3.5C1 14.4 2.6 16 4.5 16H8v-3H5l4.5-4.5L14 13h-3v3h3.5c1.9 0 3.5-1.6 3.5-3.5 0-1.8-1.4-3.3-3.2-3.5z';
break;
case 'cloud':
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These values changed because the existing cloud was moved upwards 1px to better align with text.

path = 'M14.85 10.03c1.76.18 3.15 1.66 3.15 3.47 0 1.93-1.57 3.5-3.5 3.5h-10C2.57 17 1 15.43 1 13.5c0-1.79 1.34-3.24 3.06-3.46C4.02 9.87 4 9.69 4 9.5 4 8.12 5.12 7 6.5 7c.34 0 .66.07.95.19C8.11 5.89 9.45 5 11 5c2.21 0 4 1.79 4 4 0 .36-.06.7-.15 1.03z';
path = 'M14.9 9c1.8.2 3.1 1.7 3.1 3.5 0 1.9-1.6 3.5-3.5 3.5h-10C2.6 16 1 14.4 1 12.5 1 10.7 2.3 9.3 4.1 9 4 8.9 4 8.7 4 8.5 4 7.1 5.1 6 6.5 6c.3 0 .7.1.9.2C8.1 4.9 9.4 4 11 4c2.2 0 4 1.8 4 4 0 .4-.1.7-.1 1z';
break;
case 'columns':
path = 'M3 15h6V5H3v10zm8 0h6V5h-6v10z';
Expand Down
2 changes: 2 additions & 0 deletions components/icon-button/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
position: relative;
width: $icon-button-size; // show only icon on small breakpoints
overflow: hidden;
text-indent: 8px;

.dashicon {
display: inline-block;
Expand All @@ -17,6 +18,7 @@

@include break-medium() {
width: auto;
text-indent: 4px;
}

&:not( :disabled ):hover {
Expand Down
14 changes: 11 additions & 3 deletions edit-post/components/header/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,16 +63,24 @@
background: $dark-gray-500;
}

&.editor-post-publish-button, &.editor-post-publish-panel__toggle {
&.editor-post-switch-to-draft,
&.editor-post-preview,
&.editor-post-publish-button,
&.editor-post-publish-panel__toggle {
margin: 2px;
height: 33px;
line-height: 32px;
padding: 0 12px 2px;
}

@include break-medium() {
&.editor-post-publish-button, &.editor-post-publish-panel__toggle {
margin: 0 10px;
&.editor-post-preview {
margin: 0 3px 0 12px;
}

&.editor-post-publish-button,
&.editor-post-publish-panel__toggle {
margin: 0 12px 0 3px;
}
}
}
11 changes: 7 additions & 4 deletions editor/components/post-preview-button/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { connect } from 'react-redux';
* WordPress dependencies
*/
import { Component } from '@wordpress/element';
import { IconButton } from '@wordpress/components';
import { Button } from '@wordpress/components';
import { _x } from '@wordpress/i18n';

/**
Expand Down Expand Up @@ -105,14 +105,17 @@ export class PostPreviewButton extends Component {
const { link, isSaveable } = this.props;

return (
<IconButton
<Button
className="editor-post-preview"
isLarge
href={ link }
onClick={ this.saveForPreview }
target={ this.getWindowTarget() }
icon="visibility"
disabled={ ! isSaveable }
label={ _x( 'Preview', 'imperative verb' ) }
Copy link
Member

Choose a reason for hiding this comment

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

label is specific to IconButton, and can now be removed.

/>
>
{ _x( 'Preview', 'imperative verb' ) }
</Button>
);
}
}
Expand Down
14 changes: 9 additions & 5 deletions editor/components/post-saved-state/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,12 @@
* External dependencies
*/
import { connect } from 'react-redux';
import classnames from 'classnames';

/**
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
import { Dashicon, Button } from '@wordpress/components';
import { Dashicon, IconButton } from '@wordpress/components';

/**
* Internal dependencies
Expand Down Expand Up @@ -43,7 +42,7 @@ export function PostSavedState( { isNew, isPublished, isDirty, isSaving, isSavea
}

if ( isPublished ) {
return <PostSwitchToDraftButton className={ classnames( className, 'button-link' ) } />;
return <PostSwitchToDraftButton />;
}

if ( ! isSaveable ) {
Expand All @@ -60,10 +59,15 @@ export function PostSavedState( { isNew, isPublished, isDirty, isSaving, isSavea
}

return (
<Button className={ classnames( className, 'button-link' ) } onClick={ onSave }>
<IconButton
className="editor-post-save-draft"
onClick={ onSave }
icon="cloud-upload"
label={ __( 'Save Draft' ) }
Copy link
Member

Choose a reason for hiding this comment

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

The behavior of label in this case will apply an aria-label to the element. I guess this should probably have no practical effect other than to defer to this label in all cases (even in mobile context), which may be desirable I suppose.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was sort of the thinking behind adding this. I still feel like it may not be ideal to have two elements output here, one for mobile, one for desktop, but I can't think of a better way to make a good experience for both.

I'll leave the label in and we can revisit if need be.

>
<span className="editor-post-saved-state__mobile">{ __( 'Save' ) }</span>
<span className="editor-post-saved-state__desktop">{ __( 'Save Draft' ) }</span>
</Button>
</IconButton>
);
}

Expand Down
4 changes: 4 additions & 0 deletions editor/components/post-saved-state/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
.editor-post-save-draft {
Copy link
Member

Choose a reason for hiding this comment

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

Are these overriding styles? If so, wondering if we'd need added specificity in case the styles are applied out of the expected order.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pushed a fix to remove both this style, and the style it was (yes) overriding.

The code it overrode was this: https://github.com/WordPress/gutenberg/blob/master/components/icon-button/style.scss#L10 — the idea being that an icon button can have a label, but on mobile it should always just be the icon only. By applying a width and then removing that width past the mobile breakpoint, the labels would become visible as soon as there was screen realestate. Smarts that I thought were useful for the IconButton component.

But perhaps that was too smart. In the case of this PR, I wanted the Save label to be visible even on mobile, as the icon can't stand on its own, and the autosave feature isn't yet on par with other web-apps so you still need to press that button every once in a while.

Also, removing the width doesn't seem to have had any negative effects as far as I can tell. So I removed it.

width: auto;
}

.editor-post-saved-state {
display: flex;
align-items: center;
Expand Down
10 changes: 5 additions & 5 deletions editor/components/post-switch-to-draft-button/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { connect } from 'react-redux';
/**
* WordPress dependencies
*/
import { Button } from '@wordpress/components';
import { IconButton } from '@wordpress/components';
import { __ } from '@wordpress/i18n';

/**
Expand All @@ -18,7 +18,7 @@ import {
isCurrentPostPublished,
} from '../../store/selectors';

function PostSwitchToDraftButton( { className, isSaving, isPublished, onClick } ) {
function PostSwitchToDraftButton( { isSaving, isPublished, onClick } ) {
if ( ! isPublished ) {
return null;
}
Expand All @@ -31,14 +31,14 @@ function PostSwitchToDraftButton( { className, isSaving, isPublished, onClick }
};

return (
<Button
className={ className }
<IconButton
className="editor-post-switch-to-draft"
isLarge
onClick={ onSwitch }
disabled={ isSaving }
>
{ __( 'Switch to Draft' ) }
</Button>
</IconButton>
);
}

Expand Down