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

Jump to bottom

Chrome: Replace the publish dropdown by a sidebar panel #4119

Merged
merged 5 commits into from
Dec 22, 2017
Merged

Chrome: Replace the publish dropdown by a sidebar panel #4119

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6c8ddf2
Chrome: Replace the publish dropdown by a sidebar panel
youknowriad Dec 21, 2017
131ab94
Chrome: Allow multiple sidebars
youknowriad Dec 21, 2017
100e210
Chrome: Slide in animation for the publish sidebar
youknowriad Dec 21, 2017
ae83845
Chrome: fix hasOpenSidebar selector
youknowriad Dec 21, 2017
a8c3906
Chrome: Fix animation speed of the publish sidebar
youknowriad Dec 21, 2017
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
20 changes: 9 additions & 11 deletions editor/components/block-settings-menu/block-inspector-button.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,34 +13,32 @@ import { IconButton, withSpokenMessages } from '@wordpress/components';
/**
* Internal dependencies
*/
import { isEditorSidebarOpened, getActivePanel } from '../../store/selectors';
import { getActivePanel, isSidebarOpened } from '../../store/selectors';
import { toggleSidebar, setActivePanel } from '../../store/actions';

export function BlockInspectorButton( {
isSidebarOpened,
isDefaultSidebarOpened,
panel,
onToggleSidebar,
onOpenSidebar,
onShowInspector,
onClick = noop,
small = false,
speak,
} ) {
const toggleInspector = () => {
onShowInspector();
if ( ! isSidebarOpened || ( isSidebarOpened && panel === 'block' ) ) {
onToggleSidebar();
}
onOpenSidebar( undefined, true );
};

const speakMessage = () => {
if ( ! isSidebarOpened || ( isSidebarOpened && panel !== 'block' ) ) {
if ( ! isDefaultSidebarOpened || ( isDefaultSidebarOpened && panel !== 'block' ) ) {
speak( __( 'Additional settings are now available in the Editor advanced settings sidebar' ) );
} else {
speak( __( 'Advanced settings closed' ) );
}
};

const label = ( isSidebarOpened && panel === 'block' ) ? __( 'Hide Advanced Settings' ) : __( 'Show Advanced Settings' );
const label = ( isDefaultSidebarOpened && panel === 'block' ) ? __( 'Hide Advanced Settings' ) : __( 'Show Advanced Settings' );

return (
<IconButton
Expand All @@ -56,15 +54,15 @@ export function BlockInspectorButton( {

export default connect(
( state ) => ( {
isSidebarOpened: isEditorSidebarOpened( state ),
isDefaultSidebarOpened: isSidebarOpened( state ),
panel: getActivePanel( state ),
} ),
( dispatch ) => ( {
onShowInspector() {
dispatch( setActivePanel( 'block' ) );
},
onToggleSidebar() {
dispatch( toggleSidebar() );
onOpenSidebar() {
dispatch( toggleSidebar( undefined, true ) );
},
} )
)( withSpokenMessages( BlockInspectorButton ) );
95 changes: 33 additions & 62 deletions editor/edit-post/header/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { connect } from 'react-redux';
*/
import { __ } from '@wordpress/i18n';
import { IconButton } from '@wordpress/components';
import { Component } from '@wordpress/element';

/**
* Internal dependencies
Expand All @@ -18,77 +17,49 @@ import {
PostPreviewButton,
PostSavedState,
PostPublishPanelToggle,
PostPublishPanel,
} from '../../components';
import EllipsisMenu from './ellipsis-menu';
import HeaderToolbar from './header-toolbar';
import { isEditorSidebarOpened } from '../../store/selectors';
import { isSidebarOpened } from '../../store/selectors';
import { toggleSidebar } from '../../store/actions';

class Header extends Component {
constructor() {
super( ...arguments );
this.state = {
isPublishPanelOpened: false,
};
this.togglePublishPanel = this.togglePublishPanel.bind( this );
this.closePublishPanel = this.closePublishPanel.bind( this );
}

togglePublishPanel() {
this.setState( ( state ) => ( {
isPublishPanelOpened: ! state.isPublishPanelOpened,
} ) );
}

closePublishPanel() {
this.setState( {
isPublishPanelOpened: false,
} );
}

render() {
const { onToggleSidebar, isSidebarOpened } = this.props;
return (
<div
role="region"
aria-label={ __( 'Editor toolbar' ) }
className="editor-header"
tabIndex="-1"
>
<HeaderToolbar />
function Header( { onToggleSidebar, isDefaultSidebarOpened, isPublishSidebarOpened } ) {
return (
<div
role="region"
aria-label={ __( 'Editor toolbar' ) }
className="editor-header"
tabIndex="-1"
>
<HeaderToolbar />
{ ! isPublishSidebarOpened && (
<div className="editor-header__settings">
{ ! this.state.isPublishPanelOpened && [
<PostSavedState key="saved-state" />,
<PostPreviewButton key="preview-button" />,
<PostPublishPanelToggle
key="publish-button"
isOpen={ this.state.isPublishPanelOpened }
onToggle={ this.togglePublishPanel }
/>,
<IconButton
key="sidebar-toggle"
icon="admin-generic"
onClick={ onToggleSidebar }
isToggled={ isSidebarOpened }
label={ __( 'Settings' ) }
aria-expanded={ isSidebarOpened }
/>,
<EllipsisMenu key="ellipsis-menu" />,
] }

{ this.state.isPublishPanelOpened && <PostPublishPanel onClose={ this.closePublishPanel } /> }
<PostSavedState />
<PostPreviewButton />
<PostPublishPanelToggle
isOpen={ isPublishSidebarOpened }
onToggle={ () => onToggleSidebar( 'publish' ) }
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor, but we could extend mapDispatchToProps:

{
  onToggleSidebar: toggleSidebar,
  onTogglePublishSidebar: toggleSidebar.bind( null, 'publish' ),
}

and replace the prop here with onToggle={ onTogglePublishSidebar }.

/>
<IconButton
icon="admin-generic"
onClick={ () => onToggleSidebar() }
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the function need to be bound? Perhaps this is an artefact of something else? I'd expect onClick={ onToggleSidebar }.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It needs to be to avoid passing the event as argument to the action creator :)

Copy link
Contributor

Choose a reason for hiding this comment

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

facepalm — obviously. I mean, you could just do some insane stuff in mapDispatch like toggleSidebar.bind( null, undefined, false ), but that's just idiotic. 😄 The arrow is naturally better. exits slowly

isToggled={ isDefaultSidebarOpened }
label={ __( 'Settings' ) }
aria-expanded={ isDefaultSidebarOpened }
/>
<EllipsisMenu key="ellipsis-menu" />
</div>
</div>
);
}
) }
</div>
);
}

export default connect(
( state ) => ( {
isSidebarOpened: isEditorSidebarOpened( state ),
isDefaultSidebarOpened: isSidebarOpened( state ),
isPublishSidebarOpened: isSidebarOpened( state, 'publish' ),
} ),
( dispatch ) => ( {
onToggleSidebar: () => dispatch( toggleSidebar() ),
} )
{
onToggleSidebar: toggleSidebar,
}
)( Header );
22 changes: 0 additions & 22 deletions editor/edit-post/header/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,25 +102,3 @@
}
}
}

.editor-header .editor-post-publish-panel {
position: fixed;
z-index: z-index( '.editor-layout .editor-post-publish-panel' );
top: $admin-bar-height-big;
bottom: 0;
right: 0;
left: 0;
overflow: auto;
position: fixed;

@include break-medium() {
top: $admin-bar-height;
left: auto;
width: $sidebar-width;
border-left: 1px solid $light-gray-500;
}
}

.editor-header .editor-post-publish-panel__header-publish-button {
margin-right: 52px; // This number is approximative to keep the publish button at the same position when opening the panel
}
32 changes: 26 additions & 6 deletions editor/edit-post/layout/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,34 @@ import Sidebar from '../sidebar';
import TextEditor from '../modes/text-editor';
import VisualEditor from '../modes/visual-editor';
import DocumentTitle from '../document-title';
import { MetaBoxes, AutosaveMonitor, UnsavedChangesWarning, EditorNotices } from '../../components';
import {
MetaBoxes,
AutosaveMonitor,
UnsavedChangesWarning,
EditorNotices,
PostPublishPanel,
} from '../../components';
import {
getEditorMode,
isEditorSidebarOpened,
hasOpenSidebar,
isSidebarOpened,
isFeatureActive,
} from '../../store/selectors';
import { toggleSidebar } from '../../store/actions';

function Layout( { mode, isSidebarOpened, hasFixedToolbar } ) {
function Layout( {
mode,
layoutHasOpenSidebar,
isDefaultSidebarOpened,
isPublishSidebarOpened,
hasFixedToolbar,
onToggleSidebar,
} ) {
const className = classnames( 'editor-layout', {
'is-sidebar-opened': isSidebarOpened,
'is-sidebar-opened': layoutHasOpenSidebar,
'has-fixed-toolbar': hasFixedToolbar,
} );
const closePublishPanel = () => onToggleSidebar( 'publish', false );
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor, similar comment as the previous one. Since these bound action creators don't depend on component data, I'd place them in mapDispatchToProps.


return (
<div className={ className }>
Expand All @@ -51,7 +67,8 @@ function Layout( { mode, isSidebarOpened, hasFixedToolbar } ) {
<MetaBoxes location="advanced" />
</div>
</div>
{ isSidebarOpened && <Sidebar /> }
{ isDefaultSidebarOpened && <Sidebar /> }
{ isPublishSidebarOpened && <PostPublishPanel onClose={ closePublishPanel } /> }
<Popover.Slot />
</div>
);
Expand All @@ -60,7 +77,10 @@ function Layout( { mode, isSidebarOpened, hasFixedToolbar } ) {
export default connect(
( state ) => ( {
mode: getEditorMode( state ),
isSidebarOpened: isEditorSidebarOpened( state ),
layoutHasOpenSidebar: hasOpenSidebar( state ),
isDefaultSidebarOpened: isSidebarOpened( state ),
isPublishSidebarOpened: isSidebarOpened( state, 'publish' ),
hasFixedToolbar: isFeatureActive( state, 'fixedToolbar' ),
} ),
{ onToggleSidebar: toggleSidebar }
)( navigateRegions( Layout ) );
22 changes: 22 additions & 0 deletions editor/edit-post/layout/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,25 @@
-webkit-overflow-scrolling: touch;
}
}

.editor-layout .editor-post-publish-panel {
position: fixed;
z-index: z-index( '.editor-layout .editor-post-publish-panel' );
top: $admin-bar-height-big;
bottom: 0;
right: 0;
left: 0;
overflow: auto;
position: fixed;

@include break-medium() {
top: $admin-bar-height;
left: auto;
width: $sidebar-width;
border-left: 1px solid $light-gray-500;
}
}

.editor-layout .editor-post-publish-panel__header-publish-button {
margin-right: 52px; // This number is approximative to keep the publish button at the same position when opening the panel
}
11 changes: 6 additions & 5 deletions editor/edit-post/sidebar/header.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { toggleSidebar, setActivePanel } from '../../store/actions';
const SidebarHeader = ( { panel, onSetPanel, onToggleSidebar, count } ) => {
// Do not display "0 Blocks".
count = count === 0 ? 1 : count;
const closeSidebar = () => onToggleSidebar( undefined, false );

return (
<div className="components-panel__header editor-sidebar__panel-tabs">
Expand All @@ -36,7 +37,7 @@ const SidebarHeader = ( { panel, onSetPanel, onToggleSidebar, count } ) => {
{ sprintf( _n( 'Block', '%d Blocks', count ), count ) }
</button>
<IconButton
onClick={ onToggleSidebar }
onClick={ closeSidebar }
icon="no-alt"
label={ __( 'Close settings' ) }
/>
Expand All @@ -49,8 +50,8 @@ export default connect(
panel: getActivePanel( state ),
count: getSelectedBlockCount( state ),
} ),
( dispatch ) => ( {
onSetPanel: ( panel ) => dispatch( setActivePanel( panel ) ),
onToggleSidebar: () => dispatch( toggleSidebar() ),
} )
{
onSetPanel: setActivePanel,
onToggleSidebar: toggleSidebar,
}
)( SidebarHeader );
10 changes: 6 additions & 4 deletions editor/store/actions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,13 +348,15 @@ export function stopTyping() {
/**
* Returns an action object used in signalling that the user toggled the sidebar
*
* @param {Boolean} isMobile Flag indicating if we are in mobile context
* @return {Object} Action object
* @param {String} sidebar Name of the sidebar to toggle (desktop, mobile or publish)
* @param {Boolean?} force Force a sidebar state
* @return {Object} Action object
*/
export function toggleSidebar( isMobile ) {
export function toggleSidebar( sidebar, force ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Also minor, but both arguments are nouns, yet they express different things, IMO. sidebar is the object of the action, while force is circumstantial. My suggestion would be shouldForce, in keeping with the convention for booleans. (To be super strict, one could rename the former to sidebarName, but I'm fine with sidebar.)

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 think forcedValue is better than shouldForce because it represents the value and not a flag of whether to force or not

return {
type: 'TOGGLE_SIDEBAR',
isMobile,
sidebar,
force,
};
}

Expand Down
7 changes: 5 additions & 2 deletions editor/store/defaults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
export const PREFERENCES_DEFAULTS = {
mode: 'visual',
isSidebarOpened: true,
isSidebarOpenedMobile: false,
sidebars: {
desktop: true,
mobile: false,
publish: false,
},
panels: { 'post-status': true },
recentlyUsedBlocks: [],
blockUsage: {},
Expand Down
6 changes: 4 additions & 2 deletions editor/store/reducer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -512,10 +512,12 @@ export function blockInsertionPoint( state = {}, action ) {
export function preferences( state = PREFERENCES_DEFAULTS, action ) {
switch ( action.type ) {
case 'TOGGLE_SIDEBAR':
const isSidebarOpenedKey = action.isMobile ? 'isSidebarOpenedMobile' : 'isSidebarOpened';
return {
...state,
[ isSidebarOpenedKey ]: ! state[ isSidebarOpenedKey ],
sidebars: {
...state.sidebars,
[ action.sidebar ]: action.force !== undefined ? action.force : ! state.sidebars[ action.sidebar ],
},
};
case 'TOGGLE_SIDEBAR_PANEL':
return {
Expand Down
Loading