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

RNMobile - Cover Block: First iteration #19722

Merged
merged 26 commits into from
Feb 26, 2020
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
51f8935
RNMobile - Cover Block - First iteration
Jan 17, 2020
c4bdb88
Remove allowed blocks
Jan 22, 2020
ac30632
Fix content height
Jan 22, 2020
e507dcd
Merge with master
Jan 23, 2020
970c666
RNMobile - Update Heading and Paragraph blocks to use __experimentalU…
Jan 24, 2020
419abf9
RNMobile - Cover block: import correct icon
Jan 24, 2020
cd3baae
RNMobile - Cover block - Default color text for InnerBlocks
Jan 24, 2020
7cee335
Mock use select
Jan 26, 2020
dcc1875
Merge branch 'master' of github.com:WordPress/gutenberg into rnmobile…
Jan 29, 2020
eb6fc3b
RNMobile - Cover block - Use memo from element, don't set min height …
Jan 29, 2020
96ca747
Merge branch 'master' of github.com:WordPress/gutenberg into rnmobile…
Feb 3, 2020
e06c903
RNMobile - Cover Block - Prettier and get opacity logic improvement
Feb 3, 2020
a4dfbdd
Cover block: Move onCoverSelectMedia to a utils file to be shared wit…
Feb 3, 2020
5305373
RNMobile - Cover block: don't set fontSize by default since it is not…
Feb 3, 2020
e71ee48
RNMobile - ImageWithFocalPoint - Move container logic to component an…
Feb 3, 2020
21eb57c
Merge branch 'master' of github.com:WordPress/gutenberg into rnmobile…
Feb 11, 2020
a61cbb6
RNMobile - Cover block: move attributesFromMedia to shared file, rena…
Feb 11, 2020
d7875e6
RNMobile - Enable Cover block for testing
Feb 11, 2020
63767ef
Merge branch 'master' of github.com:WordPress/gutenberg into rnmobile…
Feb 14, 2020
4f7d25a
RNMobile - Cover block - Allow changing the image and fix blocks with…
Feb 14, 2020
225e2bb
RNMobile - Cover - Fallback for theme colors
Feb 17, 2020
7a9341c
RNMobile - Cover - Fix paddings for cover blocks with and without inn…
Feb 18, 2020
ad28977
RNMobile - Cover - Overlay styles as a const instead of a function
Feb 24, 2020
b4b9a68
Merge branch 'master' of github.com:WordPress/gutenberg into rnmobile…
Feb 24, 2020
b343b2c
RNMobile - Cover block - Remove duplicated export
Feb 24, 2020
8bebf5e
RNMobile - Paragraph - Test mock import
Feb 24, 2020
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
1 change: 1 addition & 0 deletions packages/block-editor/src/components/index.native.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// Block Creation Components
export * from './gradients';
export { default as BlockAlignmentToolbar } from './block-alignment-toolbar';
export { default as BlockControls } from './block-controls';
export { default as BlockEdit, useBlockEditContext } from './block-edit';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ function MediaPlaceholder( props ) {
labels = {},
icon,
onSelect,
onlyMediaLibrary,
isAppender,
disableMediaButtons,
getStylesFromColorScheme,
Expand Down Expand Up @@ -115,6 +116,7 @@ function MediaPlaceholder( props ) {
<MediaUpload
allowedTypes={ allowedTypes }
onSelect={ setMedia }
onlyMediaLibrary={ onlyMediaLibrary }
multiple={ multiple }
render={ ( { open, getMediaOptions } ) => {
return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ const siteLibrarySource = {
label: __( 'WordPress Media Library' ),
types: [ MEDIA_TYPE_IMAGE, MEDIA_TYPE_VIDEO ],
icon: 'wordpress-alt',
mediaLibrary: true,
};

const internalSources = [ deviceLibrarySource, cameraImageSource, cameraVideoSource, siteLibrarySource ];
Expand Down Expand Up @@ -86,7 +87,7 @@ export class MediaUpload extends React.Component {
}

getMediaOptionsItems() {
const { allowedTypes = [], multiple = false } = this.props;
const { allowedTypes = [], multiple = false, onlyMediaLibrary } = this.props;

// disable upload sources for now when multiple flag is set
// eslint-disable-next-line no-undef
Expand All @@ -97,7 +98,7 @@ export class MediaUpload extends React.Component {
}

return this.getAllSources().filter( ( source ) => {
return allowedTypes.filter( ( allowedType ) => source.types.includes( allowedType ) ).length > 0;
return onlyMediaLibrary ? source.mediaLibrary : allowedTypes.filter( ( allowedType ) => source.types.includes( allowedType ) ).length > 0;
} ).map( ( source ) => {
return {
...source,
Expand Down Expand Up @@ -130,6 +131,7 @@ export class MediaUpload extends React.Component {
const { allowedTypes = [], onSelect, multiple = false } = this.props;
const mediaSource = this.getAllSources().filter( ( source ) => source.value === value ).shift();
const types = allowedTypes.filter( ( type ) => mediaSource.types.includes( type ) );

requestMediaPicker( mediaSource.id, types, multiple, ( media ) => {
if ( ( multiple && media ) || ( media && media.id ) ) {
onSelect( media );
Expand Down
2 changes: 1 addition & 1 deletion packages/block-library/src/cover/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ function ResizableCover( {
);
}

function onCoverSelectMedia( setAttributes ) {
export function onCoverSelectMedia( setAttributes ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is OK, but I'm wondering if it'd be better to extract this logic to a shared utils file instead, and have a attributesFromMedia function there.

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 think it is better to extract it, especially since the web file is kinda big already. a4dfbdd thanks for the name suggestion!

return ( media ) => {
if ( ! media || ! media.url ) {
setAttributes( { url: undefined, id: undefined } );
Expand Down
221 changes: 221 additions & 0 deletions packages/block-library/src/cover/edit.native.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,221 @@
/**
* External dependencies
*/
import { View } from 'react-native';

/**
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
import {
Icon,
ImageWithFocalPoint,
PanelBody,
RangeControl,
} from '@wordpress/components';
import {
InnerBlocks,
InspectorControls,
MEDIA_TYPE_IMAGE,
MediaPlaceholder,
withColors,
} from '@wordpress/block-editor';
import { compose, withPreferredColorScheme } from '@wordpress/compose';
import { withSelect } from '@wordpress/data';
import { useState, useEffect } from '@wordpress/element';
import { cover as icon } from '@wordpress/icons';

/**
* Internal dependencies
*/
import styles from './style.scss';
import { onCoverSelectMedia } from './edit.js';
import { COVER_MIN_HEIGHT, IMAGE_BACKGROUND_TYPE, VIDEO_BACKGROUND_TYPE } from './shared';

/**
* Constants
*/
const ALLOWED_MEDIA_TYPES = [ MEDIA_TYPE_IMAGE ];
const INNER_BLOCKS_TEMPLATE = [
[ 'core/paragraph', {
align: 'center',
fontSize: 'large',
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a bit concerned about this, since we still can't render or change font sizes from the app. This might lead to the user publishing something quite different from what they're seeing on mobile. This is specially worse if they add more than one paragraph, since the first one will be large by default and the second won't, but they would look the same inside the app.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh I didn't think about that 😱 good catch! For now, I'm removing setting the attribute as default until we support font sizes 5305373

placeholder: __( 'Write title…' ),
} ],
];
const COVER_MAX_HEIGHT = 1000;
const COVER_DEFAULT_HEIGHT = 300;

const Cover = ( {
attributes,
isAncestorSelected,
isParentSelected,
isSelected,
onFocus,
overlayColor,
setAttributes,
} ) => {
const {
backgroundType,
dimRatio,
focalPoint,
gradientValue,
minHeight,
url,
} = attributes;
const [ containerSize, setContainerSize ] = useState( null );
const CONTAINER_HEIGHT = minHeight || COVER_DEFAULT_HEIGHT;

// Used to set a default color for its InnerBlocks
// since there's no system to inherit styles yet
// the RichText component will check if there are
// parent styles for the current block. If there are,
// it will use that color instead.
useEffect( () => {
setAttributes( { childrenStyles: styles.defaultColor } );
}, [ setAttributes ] );

const onSelectMedia = ( media ) => {
const onSelect = onCoverSelectMedia( setAttributes );
onSelect( media );
};

const onHeightChange = ( value ) => {
if ( minHeight || value !== COVER_DEFAULT_HEIGHT ) {
setAttributes( { minHeight: value } );
}
};

const onOpactiyChange = ( value ) => {
setAttributes( { dimRatio: value } );
};

const onContainerLayout = ( event ) => {
const { height, width } = event.nativeEvent.layout;
setContainerSize( { width, height } );
};

const getOpacity = () => {
// Set opacity to 1 while video support is not available
if ( VIDEO_BACKGROUND_TYPE === backgroundType ) {
return 1;
}

if ( url ) {
return dimRatio !== 0 ? dimRatio / 100 : 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting, how is this different than just doing return dimRatio / 100?

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 seriously can't recall why I did this 😆 but yeah no need for that, updated it here e06c903 (sorry this commit has more changes due to prettier formatting)

}

return url ? 0.5 : 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

There's an if ( url ) with a return above this, so this would always return 1?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup no need for this, updated it e06c903#diff-b17d53802519ed746057fb2ee4a915edR111

};

const getOverlayStyles = () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

For this and getOverlay, what's the benefit of defining a function versus just assigning a opacity/overlayStyles const?

Copy link
Member Author

Choose a reason for hiding this comment

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

After a few changes, I updated it to be in a const within getOverlayStyles

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm still not sure why it is a function, since it's only called once from within the same component.

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right! Didn't see it in your first comment sorry about that! Updated ad28977#diff-b17d53802519ed746057fb2ee4a915edR113

return [
styles.overlay,
{
backgroundColor: overlayColor && overlayColor.color ? overlayColor.color : styles.overlay.color,
opacity: getOpacity(),
},
];
};

const placeholderIcon = <Icon icon={ icon } { ...styles.icon } />;

const controls = (
<InspectorControls>
<PanelBody title={ __( 'Dimensions' ) } >
<RangeControl
label={ __( 'Minimum height in pixels' ) }
minimumValue={ COVER_MIN_HEIGHT }
maximumValue={ COVER_MAX_HEIGHT }
separatorType={ 'none' }
value={ CONTAINER_HEIGHT }
onChange={ onHeightChange }
style={ styles.rangeCellContainer }
/>
</PanelBody>
{ url ?
<PanelBody title={ __( 'Overlay' ) } >
<RangeControl
label={ __( 'Background Opacity' ) }
minimumValue={ 0 }
maximumValue={ 100 }
separatorType={ 'none' }
value={ dimRatio }
onChange={ onOpactiyChange }
style={ styles.rangeCellContainer }
/>
</PanelBody> : null }
</InspectorControls>
);

const hasBackground = !! ( url || overlayColor.color || gradientValue );

const containerStyles = {
...( isParentSelected || isAncestorSelected ? styles.denseMediaPadding : styles.regularMediaPadding ),
...( isSelected && styles.innerPadding ),
};

if ( ! hasBackground ) {
return <MediaPlaceholder
icon={ placeholderIcon }
labels={ {
title: __( 'Cover' ),
} }
onSelect={ onSelectMedia }
onlyMediaLibrary={ true }
allowedTypes={ ALLOWED_MEDIA_TYPES }
onFocus={ onFocus }
/>;
}

return (
<View style={ containerStyles }>
{ controls }
<View
onLayout={ onContainerLayout }
style={ [ styles.backgroundContainer ] }>
<View style={ [ styles.content, { minHeight: CONTAINER_HEIGHT } ] } >
<InnerBlocks
template={ INNER_BLOCKS_TEMPLATE }
/>
</View>

<View style={ getOverlayStyles() } />

{ IMAGE_BACKGROUND_TYPE === backgroundType && (
<ImageWithFocalPoint
containerSize={ containerSize }
contentHeight={ containerSize ? containerSize.height : CONTAINER_HEIGHT }
focalPoint={ focalPoint }
url={ url }
/>
) }
</View>
</View>
);
};

export default compose( [
withColors( { overlayColor: 'background-color' } ),
withSelect( ( select, { clientId } ) => {
const {
getSelectedBlockClientId,
getBlockRootClientId,
getBlockParents,
} = select( 'core/block-editor' );

const parents = getBlockParents( clientId, true );

const selectedBlockClientId = getSelectedBlockClientId();
const isParentSelected = selectedBlockClientId && selectedBlockClientId === getBlockRootClientId( clientId );
const isAncestorSelected = selectedBlockClientId && parents.includes( selectedBlockClientId );

return {
isSelected: selectedBlockClientId === clientId,
isParentSelected,
isAncestorSelected,
};
} ),
withPreferredColorScheme,
] )( Cover );

41 changes: 41 additions & 0 deletions packages/block-library/src/cover/style.native.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
.icon {
fill: $gray-dark;
height: 24px;
width: 24px;
}

.innerPadding {
padding: $block-selected-to-content;
}

.regularMediaPadding {
padding: $block-edge-to-content;
}

.denseMediaPadding {
padding: $block-media-container-to-content;
}

.backgroundContainer {
overflow: hidden;
width: 100%;
position: relative;
}

.content {
justify-content: center;
width: 100%;
z-index: 3;
}

.overlay {
width: 100%;
height: 100%;
z-index: 2;
color: $black;
position: absolute;
}

.defaultColor {
color: $white;
}
Loading