Skip to content

Commit

Permalink
Merge mobile refactor of gallery to nested image blocks into desktop …
Browse files Browse the repository at this point in the history
…refactor PR(#31306)

* Move native v1 Gallery components to v1 directory
* Use v1 defaultColumnsNumber in native v1 gallery
* Make onFocus optional in MediaPlaceholder
* Add useInnerBlocksProps hook
* Enable __experimentalGalleryRefactor flag under __DEV__
* Add numColumns to block-list flat list
* Fix spacing
* Adjust styles to avoid appender overlap
* Add gallery caption
* Fix lint
Some of these "fixes" are simply disabling lint for the offending lines.
These currently unused variables may be used in a later PR, so I'm
leaving them in, for now, to help simplify reconciling the changes from
the former refactor PR.
* Fix sass lint
* [Mobile] - Refactor gallery - cherry pick image edit native (#31826)
* Remove duplicate / conflicting methods from performance refactor
* Use context for imageCrop and isGrouped instead of isGallery
* Remove non-existent inheritedAttributes attribute
* Remove dead code from non-existent context attributes
* Remove unused attributes prop from link settings
* Cherry-pick BlockListItem changes
Note: Since render was changed to renderContent, we should return early
from render too, when blockWidth is falsey.
* Return early from render instead of renderContent
* Cherry-pick plumb blockProps through BlockListBlock
I'm not sure yet whether it still makes sense to use blockProps in this
way. I'm going to cherry-pick the commits by file, and sort out the need
for this mechanism afterwards.
* Cherry-pick add GridItem
Since this is duplicated from the original mobile gallery code (Tiles
component), it might make sense to export it for re-use. Previously, it
was only moved, but now that we will maintain both versions, it has
become a duplicate implementation. I will defer this to a later commit.
* Cherry-pick BlockList
Similar to blockProps mentioned earlier, gridProperties will be
evaluated after cherry-picking the relevant changes, to see if there is
another mechanism that may be more appropriate.
* Cherry-pick StylePreview key change
* Cherry-pick blockProps and gridProperties in InnerBlocks
* Use sass var for galleryAppender padding
Note: This also re-adds fullWidth style, which is still being used in
both v1 and v2 mobile implementations. If this is superceded by a recent
refactor of the block width styles, it may be worth revisiting this and
removing / changing the implementation.
* Cherry-pick remaining gallery changes
Note: as before, blockProps and gridProperties should be re-evaluated in
subsequent commits, if necessary. E.g. `imageCrop` is already recieved
via context, and `isGroup` may be sufficient, eliminating the need for
`isGallery`.
* Remove numColumns
Going back over the older commits, it seems there were a two strategies
used to render the gallery images as a grid. One used the numColumns
(same as used in the Columns block), and the other using the Grid
component. This commit cleans up the unused parts of the former
approach.
* Remove blockProps
This is no longer necessary, since we are using context to pass
gallery-level attributes to the image blocks' rendering.
* Fix gallery block.json (delete extra comma)
* Remove unused imageCrop
* Add back blockWidth contentContainerStyles in block list
* Use boolean flags for variants in Platform module
These flags allow for a slightly more flexible, performant, and terse
way of branching by platform. For more details, see:
#18058 (comment)
* Use boolean Platform flags
* Only render imageSizeOptions loading spinner on web
* Add default for destructured context in image edit
This is necessary for unit tests, because they instantiate the block's
edit component directly, and so the default context is not provided.
* Temporarily hard-code experimenal gallery refactor flag to true
This will be reverted once the block settings are fetched from the REST
API. This is enabled for now for testing purposes.
* Update experiments page with warning about the mobile app version
* Changes new gallery flag name
* Updates mobile warning
* Removes the imageCount attribute
* Remove the isGrouped context
* Fixes lint issue

Co-authored-by: Antonis Lilis <antonis.lilis@automattic.com>
  • Loading branch information
mkevins and Antonis Lilis committed Aug 3, 2021
1 parent 0b9d8b2 commit 6a58538
Show file tree
Hide file tree
Showing 25 changed files with 458 additions and 142 deletions.
2 changes: 1 addition & 1 deletion lib/experiments-page.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ function gutenberg_initialize_experiments_settings() {
'gutenberg-experiments',
'gutenberg_experiments_section',
array(
'label' => __( 'Test a new gallery block that uses nested image blocks', 'gutenberg' ),
'label' => __( 'Test a new gallery block that uses nested image blocks (Warning: The new gallery is not compatible with WordPress mobile apps prior to version 18.1. If you use the mobile app, please update to the latest version to avoid content loss.)', 'gutenberg' ),
'id' => 'gutenberg-gallery-refactor',
)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import { ReadableContentView, alignmentHelpers } from '@wordpress/components';
*/
import BlockListBlock from './block';
import BlockInsertionPoint from './insertion-point';
import Grid from './grid-item';

import styles from './block-list-item.native.scss';
import { store as blockEditorStore } from '../../store';

Expand Down Expand Up @@ -104,7 +106,7 @@ export class BlockListItem extends Component {
];
}

render() {
renderContent() {
const {
blockAlignment,
clientId,
Expand All @@ -123,10 +125,6 @@ export class BlockListItem extends Component {
contentResizeMode === 'stretch' && stretchStyle;
const { isContainerRelated } = alignmentHelpers;

if ( ! blockWidth ) {
return null;
}

return (
<ReadableContentView
align={ blockAlignment }
Expand Down Expand Up @@ -162,6 +160,34 @@ export class BlockListItem extends Component {
</ReadableContentView>
);
}

render() {
const {
gridProperties,
clientId,
parentWidth,
items,
blockWidth,
} = this.props;

if ( ! blockWidth ) {
return null;
}

if ( gridProperties ) {
return (
<Grid
numOfColumns={ gridProperties.numColumns }
tileCount={ items.length }
index={ items.indexOf( clientId ) }
maxWidth={ parentWidth }
>
{ this.renderContent() }
</Grid>
);
}
return this.renderContent();
}
}

export default compose( [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,7 @@
.fullAlignmentPadding {
padding: $block-edge-to-content;
}

.gridItem {
overflow: visible;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/**
* External dependencies
*/
import { View } from 'react-native';

/**
* Internal dependencies
*/
import styles from './block-list-item.scss';

function Grid( props ) {
/**
* Since we don't have `calc()`, we must calculate our spacings here in
* order to preserve even spacing between tiles and equal width for tiles
* in a given row.
*
* In order to ensure equal sizing of tile contents, we distribute the
* spacing such that each tile has an equal "share" of the fixed spacing. To
* keep the tiles properly aligned within their rows, we calculate the left
* and right paddings based on the tile's relative position within the row.
*
* Note: we use padding instead of margins so that the fixed spacing is
* included within the relative spacing (i.e. width percentage), and
* wrapping behavior is preserved.
*
* - The left most tile in a row must have left padding of zero.
* - The right most tile in a row must have a right padding of zero.
*
* The values of these left and right paddings are interpolated for tiles in
* between. The right padding is complementary with the left padding of the
* next tile (i.e. the right padding of [tile n] + the left padding of
* [tile n + 1] will be equal for all tiles except the last one in a given
* row).
*
*/
const { numOfColumns, children, tileCount, index, maxWidth } = props;
const lastTile = tileCount - 1;
const lastRow = Math.floor( lastTile / numOfColumns );

const row = Math.floor( index / numOfColumns );
const rowLength =
row === lastRow ? ( lastTile % numOfColumns ) + 1 : numOfColumns;

return (
<View
style={ [
{
width: maxWidth / rowLength,
},
styles.gridItem,
] }
>
{ children }
</View>
);
}

export default Grid;
10 changes: 9 additions & 1 deletion packages/block-editor/src/components/block-list/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ const getStyles = (
const computedStyles = [
isStackedHorizontally && styles.horizontal,
horizontalAlignment && styles[ `is-aligned-${ horizontalAlignment }` ],
styles.overflowVisible,
];
stylesMemo[ styleName ] = computedStyles;
return computedStyles;
Expand Down Expand Up @@ -128,6 +129,7 @@ export class BlockList extends Component {
onDeleteBlock,
contentStyle,
renderAppender,
gridProperties,
} = this.props;
const { blockWidth } = this.state;
if (
Expand All @@ -136,7 +138,8 @@ export class BlockList extends Component {
this.extraData.onDeleteBlock !== onDeleteBlock ||
this.extraData.contentStyle !== contentStyle ||
this.extraData.renderAppender !== renderAppender ||
this.extraData.blockWidth !== blockWidth
this.extraData.blockWidth !== blockWidth ||
this.extraData.gridProperties !== gridProperties
) {
this.extraData = {
parentWidth,
Expand All @@ -145,6 +148,7 @@ export class BlockList extends Component {
contentStyle,
renderAppender,
blockWidth,
gridProperties,
};
}
return this.extraData;
Expand Down Expand Up @@ -312,9 +316,11 @@ export class BlockList extends Component {
onDeleteBlock,
rootClientId,
isStackedHorizontally,
blockClientIds,
parentWidth,
marginVertical = styles.defaultBlock.marginTop,
marginHorizontal = styles.defaultBlock.marginLeft,
gridProperties,
} = this.props;
const { blockWidth } = this.state;
return (
Expand All @@ -333,6 +339,8 @@ export class BlockList extends Component {
this.shouldShowInnerBlockAppender
}
blockWidth={ blockWidth }
gridProperties={ gridProperties }
items={ blockClientIds }
/>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ function StylePreview( { onPress, isActive, style, url } ) {
return (
<Animated.View
style={ [ outlineStyle, { opacity }, styles[ name ] ] }
key={ outlineStyle.borderColor }
key={ JSON.stringify( outlineStyle ) }
/>
);
} );
Expand Down
5 changes: 4 additions & 1 deletion packages/block-editor/src/components/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ export * from './colors';
export * from './gradients';
export * from './font-sizes';
export { AlignmentControl, AlignmentToolbar } from './alignment-control';
export { default as InnerBlocks } from './inner-blocks';
export {
default as InnerBlocks,
useInnerBlocksProps as __experimentalUseInnerBlocksProps,
} from './inner-blocks';
export { default as InspectorAdvancedControls } from './inspector-advanced-controls';
export { default as InspectorControls } from './inspector-controls';
export {
Expand Down
41 changes: 41 additions & 0 deletions packages/block-editor/src/components/inner-blocks/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*/
import { useSelect } from '@wordpress/data';
import { getBlockType, withBlockContentContext } from '@wordpress/blocks';
import { useRef } from '@wordpress/element';

/**
* Internal dependencies
Expand All @@ -23,6 +24,44 @@ import { BlockContextProvider } from '../block-context';
import { defaultLayout, LayoutProvider } from '../block-list/layout';
import { store as blockEditorStore } from '../../store';

/**
* This hook is used to lightly mark an element as an inner blocks wrapper
* element. Call this hook and pass the returned props to the element to mark as
* an inner blocks wrapper, automatically rendering inner blocks as children. If
* you define a ref for the element, it is important to pass the ref to this
* hook, which the hook in turn will pass to the component through the props it
* returns. Optionally, you can also pass any other props through this hook, and
* they will be merged and returned.
*
* @param {Object} props Optional. Props to pass to the element. Must contain
* the ref if one is defined.
* @param {Object} options Optional. Inner blocks options.
*
* @see https://github.com/WordPress/gutenberg/blob/master/packages/block-editor/src/components/inner-blocks/README.md
*/
export function useInnerBlocksProps( props = {}, options = {} ) {
const fallbackRef = useRef();
const { clientId } = useBlockEditContext();

const ref = props.ref || fallbackRef;
const InnerBlocks =
options.value && options.onChange
? ControlledInnerBlocks
: UncontrolledInnerBlocks;

return {
...props,
ref,
children: (
<InnerBlocks
{ ...options }
clientId={ clientId }
wrapperRef={ ref }
/>
),
};
}

/**
* InnerBlocks is a component which allows a single block to have multiple blocks
* as children. The UncontrolledInnerBlocks component is used whenever the inner
Expand Down Expand Up @@ -53,6 +92,7 @@ function UncontrolledInnerBlocks( props ) {
filterInnerBlocks,
blockWidth,
__experimentalLayout: layout = defaultLayout,
gridProperties,
} = props;

const block = useSelect(
Expand Down Expand Up @@ -86,6 +126,7 @@ function UncontrolledInnerBlocks( props ) {
onAddBlock={ onAddBlock }
onDeleteBlock={ onDeleteBlock }
filterInnerBlocks={ filterInnerBlocks }
gridProperties={ gridProperties }
blockWidth={ blockWidth }
/>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ function MediaPlaceholder( props ) {
labels = {},
icon,
onSelect,
onFocus,
__experimentalOnlyMediaLibrary,
isAppender,
disableMediaButtons,
Expand Down Expand Up @@ -175,7 +176,7 @@ function MediaPlaceholder( props ) {
accessibilityRole={ 'button' }
accessibilityHint={ accessibilityHint }
onPress={ ( event ) => {
props.onFocus( event );
onFocus?.( event );
open();
} }
>
Expand Down
3 changes: 3 additions & 0 deletions packages/block-editor/src/store/defaults.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ import {

const SETTINGS_DEFAULTS = {
...SETTINGS,
// FOR TESTING ONLY - Later, this will come from a REST API
// eslint-disable-next-line no-undef
__unstableGalleryWithImageBlocks: __DEV__,
alignWide: true,
};

Expand Down
20 changes: 8 additions & 12 deletions packages/block-library/src/gallery/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,17 +61,13 @@ const linkOptions = [
];
const ALLOWED_MEDIA_TYPES = [ 'image' ];

const PLACEHOLDER_TEXT = Platform.select( {
web: __(
'Drag images, upload new ones or select files from your library.'
),
native: __( 'ADD MEDIA' ),
} );

const MOBILE_CONTROL_PROPS_RANGE_CONTROL = Platform.select( {
web: {},
native: { type: 'stepper' },
} );
const PLACEHOLDER_TEXT = Platform.isNative
? __( 'ADD MEDIA' )
: __( 'Drag images, upload new ones or select files from your library.' );

const MOBILE_CONTROL_PROPS_RANGE_CONTROL = Platform.isNative
? { type: 'stepper' }
: {};

function GalleryEdit( props ) {
const {
Expand Down Expand Up @@ -491,7 +487,7 @@ function GalleryEdit( props ) {
hideCancelButton={ true }
/>
) }
{ ! imageSizeOptions && (
{ Platform.isWeb && ! imageSizeOptions && (
<BaseControl className={ 'gallery-image-sizes' }>
<BaseControl.VisualLabel>
{ __( 'Image size' ) }
Expand Down
6 changes: 4 additions & 2 deletions packages/block-library/src/gallery/gallery-styles.native.scss
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
.galleryTilesContainerSelected {
margin-bottom: 16px;
@import "./v1/gallery-styles.native.scss";

.galleryAppender {
padding-top: $grid-unit-20;
}

.fullWidth {
Expand Down
Loading

0 comments on commit 6a58538

Please sign in to comment.