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

Site Editor: add Fullscreen mode #20691

Merged
merged 7 commits into from
Mar 16, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
* WordPress dependencies
*/
import { Component } from '@wordpress/element';
import { withSelect } from '@wordpress/data';

export class FullscreenMode extends Component {
componentDidMount() {
Expand Down Expand Up @@ -45,6 +44,4 @@ export class FullscreenMode extends Component {
}
}

export default withSelect( ( select ) => ( {
isActive: select( 'core/edit-post' ).isFeatureActive( 'fullscreenMode' ),
} ) )( FullscreenMode );
export default FullscreenMode;
1 change: 1 addition & 0 deletions packages/block-editor/src/components/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export { default as ButtonBlockerAppender } from './button-block-appender';
export { default as ColorPalette } from './color-palette';
export { default as ColorPaletteControl } from './color-palette/control';
export { default as ContrastChecker } from './contrast-checker';
export { default as __experimentalFullscreenMode } from './fullscreen-mode';
export { default as __experimentalEditorSkeleton } from './editor-skeleton';
export { default as __experimentalGradientPicker } from './gradient-picker';
export { default as __experimentalGradientPickerControl } from './gradient-picker/control';
Expand Down
8 changes: 6 additions & 2 deletions packages/edit-post/src/components/layout/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { useSelect, useDispatch } from '@wordpress/data';
import {
BlockBreadcrumb,
__experimentalEditorSkeleton as EditorSkeleton,
__experimentalFullscreenMode as FullscreenMode,
} from '@wordpress/block-editor';
import {
Button,
Expand All @@ -38,7 +39,6 @@ import EditPostKeyboardShortcuts from '../keyboard-shortcuts';
import KeyboardShortcutHelpModal from '../keyboard-shortcut-help-modal';
import ManageBlocksModal from '../manage-blocks-modal';
import OptionsModal from '../options-modal';
import FullscreenMode from '../fullscreen-mode';
import BrowserURL from '../browser-url';
import Header from '../header';
import SettingsSidebar from '../sidebar/settings-sidebar';
Expand All @@ -57,6 +57,7 @@ function Layout() {
} = useDispatch( 'core/edit-post' );
const {
mode,
isFullscreenActive,
isRichEditingEnabled,
editorSidebarOpened,
pluginSidebarOpened,
Expand All @@ -81,6 +82,9 @@ function Layout() {
publishSidebarOpened: select(
'core/edit-post'
).isPublishSidebarOpened(),
isFullscreenActive: select( 'core/edit-post' ).isFeatureActive(
'fullscreenMode'
),
mode: select( 'core/edit-post' ).getEditorMode(),
isRichEditingEnabled: select( 'core/editor' ).getEditorSettings()
.richEditingEnabled,
Expand Down Expand Up @@ -110,7 +114,7 @@ function Layout() {

return (
<>
<FullscreenMode />
<FullscreenMode isActive={ isFullscreenActive } />
<BrowserURL />
<UnsavedChangesWarning />
<AutosaveMonitor />
Expand Down
2 changes: 1 addition & 1 deletion packages/edit-post/src/style.scss
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
$footer-height: $button-size-small;

@import "./components/fullscreen-mode/style.scss";
@import "../../block-editor/src/components/fullscreen-mode/style.scss";
Copy link
Member Author

Choose a reason for hiding this comment

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

@youknowriad I'm thinking these styles should probably be moved out of block-editor and every editor implementation should be able to specify their own styles when is-fullscreen-mode class is added by FullscreenMode component. Since both edit-post and edit-site are running in the same wp-admin context, we'd probably end up duplicating the code from it, but that might be fine. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with the moving, for the global style, maybe it's fine to just pick one as a convention.

Copy link
Member Author

@vindl vindl Mar 13, 2020

Choose a reason for hiding this comment

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

Based on our Slack discussion I moved this import to block-editor package in 9e2b8de.

@import "./components/header/style.scss";
@import "./components/header/fullscreen-mode-close/style.scss";
@import "./components/header/header-toolbar/style.scss";
Expand Down
1 change: 1 addition & 0 deletions packages/edit-site/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
"@wordpress/compose": "file:../compose",
"@wordpress/core-data": "file:../core-data",
"@wordpress/data": "file:../data",
"@wordpress/edit-post": "file:../edit-post",
"@wordpress/editor": "file:../editor",
"@wordpress/element": "file:../element",
"@wordpress/hooks": "file:../hooks",
Expand Down
71 changes: 43 additions & 28 deletions packages/edit-site/src/components/editor/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ import {
FocusReturnProvider,
} from '@wordpress/components';
import { EntityProvider } from '@wordpress/core-data';
import { __experimentalEditorSkeleton as EditorSkeleton } from '@wordpress/block-editor';
import {
__experimentalEditorSkeleton as EditorSkeleton,
__experimentalFullscreenMode as FullscreenMode,
} from '@wordpress/block-editor';
import { useViewportMatch } from '@wordpress/compose';

/**
Expand Down Expand Up @@ -47,35 +50,47 @@ function Editor( { settings: _settings } ) {
settings,
setSettings,
] );

const { isFullscreenActive } = useSelect( ( select ) => {
return {
isFullscreenActive: select( 'core/edit-site' ).isFeatureActive(
'fullscreenMode'
),
};
}, [] );

return template ? (
<SlotFillProvider>
<DropZoneProvider>
<EntityProvider kind="root" type="site">
<EntityProvider
kind="postType"
type={ settings.templateType }
id={ settings.templateId }
>
<Context.Provider value={ context }>
<FocusReturnProvider>
<EditorSkeleton
sidebar={ ! isMobile && <Sidebar /> }
header={ <Header /> }
content={
<>
<Notices />
<Popover.Slot name="block-toolbar" />
<BlockEditor />
</>
}
/>
<Popover.Slot />
</FocusReturnProvider>
</Context.Provider>
<>
<FullscreenMode isActive={ isFullscreenActive } />
<SlotFillProvider>
<DropZoneProvider>
<EntityProvider kind="root" type="site">
<EntityProvider
kind="postType"
type={ settings.templateType }
id={ settings.templateId }
>
<Context.Provider value={ context }>
<FocusReturnProvider>
<EditorSkeleton
sidebar={ ! isMobile && <Sidebar /> }
header={ <Header /> }
content={
<>
<Notices />
<Popover.Slot name="block-toolbar" />
<BlockEditor />
</>
}
/>
<Popover.Slot />
</FocusReturnProvider>
</Context.Provider>
</EntityProvider>
</EntityProvider>
</EntityProvider>
</DropZoneProvider>
</SlotFillProvider>
</DropZoneProvider>
</SlotFillProvider>
</>
) : null;
}
export default Editor;
1 change: 1 addition & 0 deletions packages/edit-site/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { render } from '@wordpress/element';
* Internal dependencies
*/
import './hooks';
import './store';
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel this config should be reused somehow as well, some options:

  • Move it to the block-editor store (doesn't make sense for me as it's just a temporary location for the component)
  • Use a store in the new package, downside is that it makes it a singleton package (another one). (option we used everywhere for the moment)

Options we didn't use anywhere yet but could be good for reusabliity

  • Export a store config from the new package and leave the responsibility to register it to the caller wp.data.registerStore( 'core/edit-post-full-screen', fullscreenStoreConfig )
  • Don't use a store, use a React context provider.

I don't think it needs to be solved in this PR, but something to think about.

cc @aduth if you have thoughts.

Copy link
Member

Choose a reason for hiding this comment

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

To clarify, by "config", do you mean the user's general preference of "full site mode" being activated or not, across different implementations of the block editor?

Copy link
Contributor

@youknowriad youknowriad Mar 13, 2020

Choose a reason for hiding this comment

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

yes. But not necessarily the persisted preference since the persisted one can be different from screen to other. (just the memory handling of the preference)

import Editor from './components/editor';

/**
Expand Down
13 changes: 13 additions & 0 deletions packages/edit-site/src/store/actions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/**
* Returns an action object used to toggle a feature flag.
*
* @param {string} feature Feature name.
*
* @return {Object} Action object.
*/
export function toggleFeature( feature ) {
return {
type: 'TOGGLE_FEATURE',
feature,
};
}
6 changes: 6 additions & 0 deletions packages/edit-site/src/store/constants.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/**
* The identifier for the data store.
*
* @type {string}
*/
export const STORE_KEY = 'core/edit-site';
32 changes: 32 additions & 0 deletions packages/edit-site/src/store/controls.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/**
* WordPress dependencies
*/
import { createRegistryControl } from '@wordpress/data';

/**
* Calls a selector using the current state.
*
* @param {string} storeName Store name.
* @param {string} selectorName Selector name.
* @param {Array} args Selector arguments.
*
* @return {Object} control descriptor.
*/
export function select( storeName, selectorName, ...args ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Where does this export get used?
I see the default export in this file gets used in registerStore in the index, but what is this function doing?

Copy link
Member Author

Choose a reason for hiding this comment

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

I copied over the scaffolding from edit-post store to speed things up so likely an unintended leftover. Will recheck and remove it if it's not necessary. I also have to look into failing CI tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed in 1ce8f0d.

return {
type: 'SELECT',
storeName,
selectorName,
args,
};
}

const controls = {
SELECT: createRegistryControl(
( registry ) => ( { storeName, selectorName, args } ) => {
return registry.select( storeName )[ selectorName ]( ...args );
}
),
};

export default controls;
5 changes: 5 additions & 0 deletions packages/edit-site/src/store/defaults.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export const PREFERENCES_DEFAULTS = {
features: {
fullscreenMode: false,
},
};
23 changes: 23 additions & 0 deletions packages/edit-site/src/store/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/**
* WordPress dependencies
*/
import { registerStore } from '@wordpress/data';

/**
* Internal dependencies
*/
import reducer from './reducer';
import * as actions from './actions';
import * as selectors from './selectors';
import controls from './controls';
import { STORE_KEY } from './constants';

const store = registerStore( STORE_KEY, {
reducer,
actions,
selectors,
controls,
persist: [ 'preferences' ],
} );

export default store;
53 changes: 53 additions & 0 deletions packages/edit-site/src/store/reducer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/**
* External dependencies
*/
import { flow } from 'lodash';

/**
* WordPress dependencies
*/
import { combineReducers } from '@wordpress/data';

/**
* Internal dependencies
*/
import { PREFERENCES_DEFAULTS } from './defaults';

/**
* Higher-order reducer creator which provides the given initial state for the
* original reducer.
*
* @param {*} initialState Initial state to provide to reducer.
*
* @return {Function} Higher-order reducer.
*/
const createWithInitialState = ( initialState ) => ( reducer ) => {
return ( state = initialState, action ) => reducer( state, action );
};

/**
* Reducer returning the user preferences.
*
* @param {Object} state Current state.
*
* @return {Object} Updated state.
*/
export const preferences = flow( [
combineReducers,
createWithInitialState( PREFERENCES_DEFAULTS ),
] )( {
features( state, action ) {
if ( action.type === 'TOGGLE_FEATURE' ) {
return {
...state,
[ action.feature ]: ! state[ action.feature ],
};
}

return state;
},
} );

export default combineReducers( {
preferences,
} );
16 changes: 16 additions & 0 deletions packages/edit-site/src/store/selectors.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/**
* External dependencies
*/
import { get } from 'lodash';

/**
* Returns whether the given feature is enabled or not.
*
* @param {Object} state Global application state.
* @param {string} feature Feature slug.
*
* @return {boolean} Is active.
*/
export function isFeatureActive( state, feature ) {
return get( state.preferences.features, [ feature ], false );
}
16 changes: 16 additions & 0 deletions packages/edit-site/src/store/test/actions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/**
* Internal dependencies
*/
import { toggleFeature } from '../actions';

describe( 'actions', () => {
describe( 'toggleFeature', () => {
it( 'should return TOGGLE_FEATURE action', () => {
const feature = 'name';
expect( toggleFeature( feature ) ).toEqual( {
type: 'TOGGLE_FEATURE',
feature,
} );
} );
} );
} );
32 changes: 32 additions & 0 deletions packages/edit-site/src/store/test/reducer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/**
* External dependencies
*/
import deepFreeze from 'deep-freeze';

/**
* Internal dependencies
*/
import { preferences } from '../reducer';
import { PREFERENCES_DEFAULTS } from '../defaults';

describe( 'state', () => {
describe( 'preferences()', () => {
it( 'should apply all defaults', () => {
const state = preferences( undefined, {} );

expect( state ).toEqual( PREFERENCES_DEFAULTS );
} );

it( 'should toggle a feature flag', () => {
const state = preferences(
deepFreeze( { features: { chicken: true } } ),
{
type: 'TOGGLE_FEATURE',
feature: 'chicken',
}
);

expect( state.features ).toEqual( { chicken: false } );
} );
} );
} );
Loading