From 70e9f8bc13a55c652fc1b61ae8f52ea4a124e9fd Mon Sep 17 00:00:00 2001 From: Drapich Piotr Date: Wed, 25 Mar 2020 18:24:59 +0100 Subject: [PATCH] [RNMobile] Reimplementation of block borders (#20769) * first working version - w/o media text * fix not visible borders * TMP media-text * move inline styles to styles file * Working version with media text * Remove unused styles * fix toolbar styles * use variable in floating toolbar style * fix cover block * fix base variables * add basic horizontal support * change approach and use negative margins * fix appender in empty group * Fix appender * fix toolbar margin * add comment and move borders spacing val to consts * fix media-text and remove unused props form withSelect in block * remove not needed overflow visible * add style mock * fix style code formatting --- packages/base-styles/_variables.scss | 9 +- .../src/components/block-list/block.native.js | 141 ++++-------------- .../components/block-list/block.native.scss | 97 +++--------- .../src/components/block-list/index.native.js | 43 ++++-- .../components/block-list/style.native.scss | 13 +- .../components/inner-blocks/index.native.js | 9 +- .../block-library/src/cover/edit.native.js | 58 +++---- .../block-library/src/cover/style.native.scss | 13 +- .../block-library/src/group/edit.native.js | 8 +- .../src/group/editor.native.scss | 4 + .../src/media-text/edit.native.js | 35 +++-- .../src/media-text/style.native.scss | 40 +++-- test/native/__mocks__/styleMock.js | 3 + 13 files changed, 190 insertions(+), 283 deletions(-) diff --git a/packages/base-styles/_variables.scss b/packages/base-styles/_variables.scss index 106652e07e095..404e8b25c87ae 100644 --- a/packages/base-styles/_variables.scss +++ b/packages/base-styles/_variables.scss @@ -83,18 +83,13 @@ $block-bg-padding--h: $block-side-ui-width + $block-side-ui-clearance; // paddin $dimmed-opacity: 1; $block-edge-to-content: 16px; +$solid-border-space: 12px; +$dashed-border-space: 6px; $block-selected-margin: 3px; $block-selected-border-width: 1px; $block-selected-padding: 0; $block-selected-child-margin: 5px; -$block-selected-child-border-width: 1px; -$block-selected-child-padding: 0; $block-selected-to-content: $block-edge-to-content - $block-selected-margin - $block-selected-border-width; -$block-selected-child-to-content: $block-selected-to-content - $block-selected-child-margin - $block-selected-child-border-width; -$block-custom-appender-to-content: $block-selected-margin - $block-selected-border-width; -$block-media-container-to-content: $block-selected-child-margin + $block-selected-border-width; -$block-selected-vertical-margin-descendant: 2 * $block-selected-to-content; -$block-selected-vertical-margin-child: $block-edge-to-content; /** diff --git a/packages/block-editor/src/components/block-list/block.native.js b/packages/block-editor/src/components/block-list/block.native.js index 2501094b59339..db30304cfeee3 100644 --- a/packages/block-editor/src/components/block-list/block.native.js +++ b/packages/block-editor/src/components/block-list/block.native.js @@ -12,7 +12,6 @@ import { withDispatch, withSelect } from '@wordpress/data'; import { compose, withPreferredColorScheme } from '@wordpress/compose'; import { getBlockType, - getUnregisteredTypeHandlerName, __experimentalGetAccessibleBlockLabel as getAccessibleBlockLabel, } from '@wordpress/blocks'; import { __ } from '@wordpress/i18n'; @@ -79,104 +78,6 @@ class BlockListBlock extends Component { ); } - applySelectedBlockStyle() { - const { hasChildren, getStylesFromColorScheme } = this.props; - - const fullSolidBorderStyle = { - // define style for full border - ...styles.fullSolidBordered, - ...getStylesFromColorScheme( - styles.solidBorderColor, - styles.solidBorderColorDark - ), - }; - - if ( hasChildren ) { - // if block has children apply style for selected parent - return { ...styles.selectedParent, ...fullSolidBorderStyle }; - } - - /* selected block is one of below: - 1. does not have children - 2. is not on root list level - 3. is an emty group block on root or nested level */ - return { ...styles.selectedLeaf, ...fullSolidBorderStyle }; - } - - applyUnSelectedBlockStyle() { - const { - hasChildren, - isParentSelected, - isAncestorSelected, - hasParent, - getStylesFromColorScheme, - isLastBlock, - } = this.props; - - // if block does not have parent apply neutral or full - // margins depending if block has children or not - if ( ! hasParent ) { - return hasChildren ? styles.neutral : styles.full; - } - - if ( isParentSelected ) { - // parent of a block is selected - const dashedBorderStyle = { - // define style for dashed border - ...styles.dashedBordered, - ...getStylesFromColorScheme( - styles.dashedBorderColor, - styles.dashedBorderColorDark - ), - }; - - // return apply childOfSelected or childOfSelectedLeaf - // margins depending if block has children or not - return { - ...( hasChildren - ? styles.childOfSelected - : styles.childOfSelectedLeaf ), - ...dashedBorderStyle, - ...( ! isLastBlock && styles.marginVerticalChild ), - }; - } - - if ( isAncestorSelected ) { - // ancestor of a block is selected - return { - ...styles.descendantOfSelectedLeaf, - ...( hasChildren && { - ...styles.marginHorizontalNone, - ...styles.marginVerticalNone, - } ), - ...( ! isLastBlock && styles.marginVerticalDescendant ), - }; - } - - // if none of above condition are met return apply neutral or full - // margins depending if block has children or not - return hasChildren ? styles.neutral : styles.full; - } - - applyBlockStyle() { - const { isSelected, isDimmed } = this.props; - - return [ - isSelected - ? this.applySelectedBlockStyle() - : this.applyUnSelectedBlockStyle(), - isDimmed && styles.dimmed, - ]; - } - - applyToolbarStyle() { - const { hasChildren, isUnregisteredBlock } = this.props; - - if ( ! hasChildren || isUnregisteredBlock ) { - return styles.neutralToolbar; - } - } - render() { const { attributes, @@ -188,10 +89,15 @@ class BlockListBlock extends Component { order, title, parentId, + isDimmed, isTouchable, hasParent, + isParentSelected, onSelect, showFloatingToolbar, + getStylesFromColorScheme, + marginVertical, + marginHorizontal, } = this.props; const accessibilityLabel = getAccessibleBlockLabel( @@ -225,8 +131,33 @@ class BlockListBlock extends Component { + { isSelected && ( + + ) } + { isParentSelected && ( + + ) } { isValid ? ( this.getBlockForType() ) : ( @@ -235,7 +166,7 @@ class BlockListBlock extends Component { icon={ icon } /> ) } - + { isSelected && ( ) } @@ -259,16 +190,13 @@ export default compose( [ getBlockRootClientId, getLowestCommonAncestorWithSelectedBlock, getBlockParents, - getBlockCount, } = select( 'core/block-editor' ); const order = getBlockIndex( clientId, rootClientId ); const isSelected = isBlockSelected( clientId ); - const isLastBlock = order === getBlockCount( rootClientId ) - 1; const block = __unstableGetBlockWithoutInnerBlocks( clientId ); const { name, attributes, isValid } = block || {}; - const isUnregisteredBlock = name === getUnregisteredTypeHandlerName(); const blockType = getBlockType( name || 'core/missing' ); const title = blockType.title; const icon = blockType.icon; @@ -292,8 +220,6 @@ export default compose( [ ? parents[ commonAncestorIndex ] : parents[ parents.length - 1 ]; - const hasChildren = - ! isUnregisteredBlock && !! getBlockCount( clientId ); const hasParent = !! parentId; const isParentSelected = selectedBlockClientId && selectedBlockClientId === parentId; @@ -329,18 +255,15 @@ export default compose( [ title, attributes, blockType, - isLastBlock, isSelected, isValid, parentId, isParentSelected, firstToSelectId, - hasChildren, hasParent, isAncestorSelected, isTouchable, isDimmed, - isUnregisteredBlock, showFloatingToolbar, }; } ), diff --git a/packages/block-editor/src/components/block-list/block.native.scss b/packages/block-editor/src/components/block-list/block.native.scss index 85bae3528b130..0d1c60fe4f1f1 100644 --- a/packages/block-editor/src/components/block-list/block.native.scss +++ b/packages/block-editor/src/components/block-list/block.native.scss @@ -2,18 +2,6 @@ flex: 1 1 auto; } -.fullSolidBordered { - border-width: $block-selected-border-width; - border-radius: 4px; - border-style: solid; -} - -.dashedBordered { - border-width: $block-selected-child-border-width; - border-radius: 2px; - border-style: dashed; -} - .solidBorderColor { border-color: $blue-wordpress; } @@ -34,31 +22,6 @@ opacity: $dimmed-opacity; } -.horizontalSpaceNone { - padding-left: 0; - padding-right: 0; - margin-left: 0; - margin-right: 0; -} - -.marginHorizontalNone { - margin-left: 0; - margin-right: 0; -} - -.marginVerticalDescendant { - margin-bottom: $block-selected-vertical-margin-descendant; -} - -.marginVerticalChild { - margin-bottom: $block-selected-vertical-margin-child; -} - -.marginVerticalNone { - margin-top: 0; - margin-bottom: 0; -} - .blockTitle { background-color: $gray; padding-left: 8px; @@ -66,44 +29,6 @@ padding-bottom: 4px; } -.neutral { - margin: 0; - border: 0; - padding: 0; -} - -.full { - margin: $block-edge-to-content; - border: 0; - padding: 0; -} - -.selectedLeaf { - margin: $block-selected-margin; - padding-left: $block-selected-to-content; - padding-right: $block-selected-to-content; - padding-top: $block-selected-to-content; -} - -.selectedParent { - margin: $block-selected-margin; - padding: 0; -} - -.childOfSelected { - margin: $block-selected-child-margin; - padding: 0; -} - -.childOfSelectedLeaf { - margin: $block-selected-child-margin; - padding: $block-selected-child-to-content; -} - -.descendantOfSelectedLeaf { - margin: $block-selected-child-to-content; -} - .aztec_container { flex: 1; } @@ -137,3 +62,25 @@ margin-left: -$block-edge-to-content; margin-right: -$block-edge-to-content; } + +.solidBorder { + position: absolute; + top: -$solid-border-space; + bottom: 0; + left: -$solid-border-space; + right: -$solid-border-space; + border-width: $block-selected-border-width; + border-radius: 4px; + border-style: solid; +} + +.dashedBorder { + position: absolute; + top: -$dashed-border-space; + bottom: -$dashed-border-space; + left: -$dashed-border-space; + right: -$dashed-border-space; + border-width: $block-selected-border-width; + border-radius: 2px; + border-style: dashed; +} diff --git a/packages/block-editor/src/components/block-list/index.native.js b/packages/block-editor/src/components/block-list/index.native.js index b43b6f82c94e0..53d696e5d00e6 100644 --- a/packages/block-editor/src/components/block-list/index.native.js +++ b/packages/block-editor/src/components/block-list/index.native.js @@ -72,13 +72,15 @@ export class BlockList extends Component { const { shouldShowInsertionPointBefore } = this.props; const willShowInsertionPoint = shouldShowInsertionPointBefore(); // call without the client_id argument since this is the appender return ( - - - + + + + + ); } @@ -98,6 +100,8 @@ export class BlockList extends Component { isRootList, shouldShowInsertionPointBefore, shouldShowInsertionPointAfter, + marginVertical = styles.defaultBlock.marginTop, + marginHorizontal = styles.defaultBlock.marginLeft, } = this.props; const { blockToolbar, blockBorder, headerToolbar } = styles; @@ -105,9 +109,16 @@ export class BlockList extends Component { const forceRefresh = shouldShowInsertionPointBefore || shouldShowInsertionPointAfter; + const containerStyle = { + flex: isRootList ? 1 : 0, + // We set negative margin in the parent to remove the edge spacing between parent block and child block in ineer blocks + marginVertical: isRootList ? 0 : -marginVertical, + marginHorizontal: isRootList ? 0 : -marginHorizontal, + }; + return ( { this.shouldShowInnerBlockAppender() && ( - + { ! templateInProcess && ( ); - const containerStyles = [ - hasChildren && ! isParentSelected && styles.regularMediaPadding, - hasChildren && isParentSelected && styles.innerPadding, - ]; - const background = ( openMediaOptions, getMediaOptions ) => ( + + { controls } - - - - - - { /* We don't render overlay on top of gradient */ } - { ! gradientValue && ( - - ) } - { - return background( open, getMediaOptions ); - } } - /> + + + + { /* We don't render overlay on top of gradient */ } + { ! gradientValue && ( + + ) } + + { + return background( open, getMediaOptions ); + } } + /> ); }; @@ -270,15 +260,11 @@ const Cover = ( { export default compose( [ withColors( { overlayColor: 'background-color' } ), withSelect( ( select, { clientId } ) => { - const { getSelectedBlockClientId, getBlockCount } = select( - 'core/block-editor' - ); + const { getSelectedBlockClientId } = select( 'core/block-editor' ); const selectedBlockClientId = getSelectedBlockClientId(); - const hasChildren = getBlockCount( clientId ); return { - hasChildren, isParentSelected: selectedBlockClientId === clientId, }; } ), diff --git a/packages/block-library/src/cover/style.native.scss b/packages/block-library/src/cover/style.native.scss index 2141f715a2b7e..b1e7b5bad9257 100644 --- a/packages/block-library/src/cover/style.native.scss +++ b/packages/block-library/src/cover/style.native.scss @@ -8,18 +8,6 @@ fill: $white; } -.innerPadding { - padding: $block-selected-to-content; -} - -.regularMediaPadding { - padding: $block-edge-to-content; -} - -.denseMediaPadding { - padding: $block-media-container-to-content; -} - .backgroundContainer { overflow: hidden; width: 100%; @@ -29,6 +17,7 @@ .content { justify-content: center; width: 100%; + padding: $block-edge-to-content; z-index: 3; } diff --git a/packages/block-library/src/group/edit.native.js b/packages/block-library/src/group/edit.native.js index a1b261f3340c6..257ba8b4e628c 100644 --- a/packages/block-library/src/group/edit.native.js +++ b/packages/block-library/src/group/edit.native.js @@ -33,9 +33,11 @@ function GroupEdit( { hasInnerBlocks, isSelected, getStylesFromColorScheme } ) { } return ( - + + + ); } diff --git a/packages/block-library/src/group/editor.native.scss b/packages/block-library/src/group/editor.native.scss index 2f426176fe22a..7f8a9ce132246 100644 --- a/packages/block-library/src/group/editor.native.scss +++ b/packages/block-library/src/group/editor.native.scss @@ -22,3 +22,7 @@ margin-left: 0; margin-right: 0; } + +.innerBlocks { + margin-bottom: $block-edge-to-content; +} diff --git a/packages/block-library/src/media-text/edit.native.js b/packages/block-library/src/media-text/edit.native.js index 3a582686c7c7b..10d948372c4c6 100644 --- a/packages/block-library/src/media-text/edit.native.js +++ b/packages/block-library/src/media-text/edit.native.js @@ -184,8 +184,6 @@ class MediaTextEdit extends Component { backgroundColor, setAttributes, isSelected, - isParentSelected, - isAncestorSelected, } = this.props; const { isStackedOnMobile, @@ -202,11 +200,14 @@ class MediaTextEdit extends Component { : this.state.mediaWidth || mediaWidth; const widthString = `${ temporaryMediaWidth }%`; - const innerBlockContainerStyle = ! shouldStack && { - ...styles.paddingHorizontalNone, - ...( mediaPosition === 'right' && styles.innerPaddingMediaOnRight ), - ...( mediaPosition === 'left' && styles.innerPaddingMediaOnLeft ), - }; + const innerBlockContainerStyle = ! shouldStack + ? styles.innerBlock + : { + ...( mediaPosition === 'left' + ? styles.innerBlockStackMediaLeft + : styles.innerBlockStackMediaRight ), + }; + const containerStyles = { ...styles[ 'wp-block-media-text' ], ...styles[ @@ -215,20 +216,26 @@ class MediaTextEdit extends Component { ...( mediaPosition === 'right' ? styles[ 'has-media-on-the-right' ] : {} ), - ...( shouldStack ? styles[ 'is-stacked-on-mobile' ] : {} ), + ...( shouldStack && styles[ 'is-stacked-on-mobile' ] ), ...( shouldStack && mediaPosition === 'right' ? styles[ 'is-stacked-on-mobile.has-media-on-the-right' ] : {} ), + ...( isSelected && styles[ 'is-selected' ] ), backgroundColor: backgroundColor.color, }; + const innerBlockWidth = shouldStack ? 100 : 100 - temporaryMediaWidth; const innerBlockWidthString = `${ innerBlockWidth }%`; - const mediaContainerStyle = { - ...( isParentSelected || isAncestorSelected - ? styles.denseMediaPadding - : styles.regularMediaPadding ), - ...( isSelected && styles.innerPadding ), - }; + + const mediaContainerStyle = shouldStack + ? { + ...( mediaPosition === 'left' && styles.mediaStackLeft ), + ...( mediaPosition === 'right' && styles.mediaStackRight ), + } + : { + ...( mediaPosition === 'left' && styles.mediaLeft ), + ...( mediaPosition === 'right' && styles.mediaRight ), + }; const toolbarControls = [ { diff --git a/packages/block-library/src/media-text/style.native.scss b/packages/block-library/src/media-text/style.native.scss index 75fb16c740df8..4e8cca364896d 100644 --- a/packages/block-library/src/media-text/style.native.scss +++ b/packages/block-library/src/media-text/style.native.scss @@ -1,3 +1,5 @@ +$media-to-text: 12px; + .wp-block-media-text { display: flex; align-items: flex-start; @@ -28,6 +30,10 @@ align-items: flex-end; } +.is-selected { + padding-bottom: 8; +} + .content { flex: 1; } @@ -104,27 +110,35 @@ color: $gray-dark; } -.innerPadding { - padding: $block-selected-to-content; +// Inner blocks STACK +.innerBlockStackMediaLeft { + margin-top: $media-to-text; +} + +.innerBlockStackMediaRight { + margin-bottom: $media-to-text; } -.innerPaddingMediaOnLeft { - padding-right: $block-selected-to-content; +// Inner blocks +.innerBlock { + padding-right: $media-to-text; + padding-left: $media-to-text; } -.innerPaddingMediaOnRight { - padding-left: $block-selected-to-content; +// Media STACK +.mediaStackLeft { + margin-bottom: $media-to-text; } -.paddingHorizontalNone { - padding-left: 0; - padding-right: 0; +.mediaStackRight { + margin-top: $media-to-text; } -.regularMediaPadding { - padding: $block-edge-to-content; +// Media +.mediaLeft { + padding-right: $media-to-text; } -.denseMediaPadding { - padding: $block-media-container-to-content; +.mediaRight { + padding-left: $media-to-text; } diff --git a/test/native/__mocks__/styleMock.js b/test/native/__mocks__/styleMock.js index 6a1481e6fe388..55fa4dd0a954c 100644 --- a/test/native/__mocks__/styleMock.js +++ b/test/native/__mocks__/styleMock.js @@ -90,4 +90,7 @@ module.exports = { blockBorder: { width: 1, }, + defaultBlock: { + marginTop: 16, + }, };