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

Quick UX fixes #3611

Merged
merged 13 commits into from
Oct 25, 2019
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,14 @@ import {
* @param {Array} props.backgroundColors Current background colors.
* @param {Function} props.setAttributes setAttributes callback.
* @param {number} props.overlayOpacity Overlay opacity.
* @param {number} props.hasMedia Whether the page has a media item set.
* @return {ReactElement} Component.
*/
const BackgroundColorSettings = ( { backgroundColors, setAttributes, overlayOpacity } ) => {
const BackgroundColorSettings = ( { backgroundColors, setAttributes, overlayOpacity, hasMedia } ) => {
const hasColors = backgroundColors
swissspidy marked this conversation as resolved.
Show resolved Hide resolved
.map( ( color ) => color && Object.values( color ).filter( Boolean ).length )
.filter( Boolean ).length > 0;

const removeBackgroundColor = ( index ) => {
backgroundColors.splice( index, 1 );
setAttributes( { backgroundColors: JSON.stringify( backgroundColors ) } );
Expand All @@ -35,7 +40,11 @@ const BackgroundColorSettings = ( { backgroundColors, setAttributes, overlayOpac
backgroundColors[ index ] = {
color: value,
};
setAttributes( { backgroundColors: JSON.stringify( backgroundColors ) } );

setAttributes( {
backgroundColors: JSON.stringify( backgroundColors ),
overlayOpacity: ! hasColors && overlayOpacity === 100 && hasMedia ? 50 : overlayOpacity,
} );
};

const getOverlayColorSettings = () => {
Expand All @@ -46,7 +55,7 @@ const BackgroundColorSettings = ( { backgroundColors, setAttributes, overlayOpac
onChange: ( value ) => {
setBackgroundColors( value, 0 );
},
label: __( 'Color', 'amp' ),
label: __( 'Background Color', 'amp' ),
},
];
}
Expand All @@ -61,7 +70,7 @@ const BackgroundColorSettings = ( { backgroundColors, setAttributes, overlayOpac
setBackgroundColors( value, index );
},
/* translators: %s: color number */
label: useNumberedLabels ? sprintf( __( 'Color %s', 'amp' ), index + 1 ) : __( 'Color', 'amp' ),
label: useNumberedLabels ? sprintf( __( 'Color %s', 'amp' ), index + 1 ) : __( 'Background Color', 'amp' ),
};
} );

Expand All @@ -70,7 +79,7 @@ const BackgroundColorSettings = ( { backgroundColors, setAttributes, overlayOpac

return (
<PanelColorSettings
title={ __( 'Background Color', 'amp' ) }
title={ __( 'Color Settings', 'amp' ) }
colorSettings={ getOverlayColorSettings() }
>
<p>
Expand All @@ -90,6 +99,7 @@ const BackgroundColorSettings = ( { backgroundColors, setAttributes, overlayOpac
</Button>
}
</p>
{ hasColors &&
<RangeControl
label={ __( 'Opacity', 'amp' ) }
value={ overlayOpacity }
Expand All @@ -99,6 +109,7 @@ const BackgroundColorSettings = ( { backgroundColors, setAttributes, overlayOpac
step={ 5 }
required
/>
}
</PanelColorSettings>
);
};
Expand All @@ -107,6 +118,7 @@ BackgroundColorSettings.propTypes = {
backgroundColors: PropTypes.array,
overlayOpacity: PropTypes.number,
setAttributes: PropTypes.func.isRequired,
hasMedia: PropTypes.bool.isRequired,
};

export default BackgroundColorSettings;
1 change: 1 addition & 0 deletions assets/src/stories-editor/blocks/amp-story-page/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ const PageEdit = ( {
backgroundColors={ bgColors }
setAttributes={ setAttributes }
overlayOpacity={ overlayOpacity }
hasMedia={ Boolean( mediaUrl ) }
/>
<BackgroundMediaSettings
allowedBackgroundMediaTypes={ allowedBackgroundMediaTypes }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,6 @@ export default createHigherOrderComponent(
<InspectorControls>
<PanelColorSettings
title={ __( 'Color Settings', 'amp' ) }
initialOpen={ false }
colorSettings={ [
{
value: backgroundColor.color,
Expand All @@ -511,7 +510,7 @@ export default createHigherOrderComponent(
fontSize: fontSize.size,
} }
/>
{ ! excludeOpacity && (
{ ( ! excludeOpacity && backgroundColor.color ) && (
<RangeControl
label={ __( 'Opacity', 'amp' ) }
value={ opacity }
Expand Down

This file was deleted.

1 change: 0 additions & 1 deletion assets/src/stories-editor/components/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ export { default as withCroppedFeaturedImage } from './with-cropped-featured-ima
export { default as withHasSelectedInnerBlock } from './higher-order/with-has-selected-inner-block';
export { default as withPageNumber } from './higher-order/with-page-number';
export { default as withContextMenu } from './higher-order/with-context-menu';
export { default as withStoryFeaturedImageNotice } from './higher-order/with-story-featured-image-notice';
export { default as withEditFeaturedImage } from './with-edit-featured-image';
export { default as withCustomVideoBlockEdit } from './with-custom-video-block-edit';
export { default as CustomVideoBlockEdit } from './custom-video-block-edit';
Expand Down
2 changes: 0 additions & 2 deletions assets/src/stories-editor/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import {
withPageNumber,
withEditFeaturedImage,
withCustomVideoBlockEdit,
withStoryFeaturedImageNotice,
withWrapperProps,
withActivePageState,
withStoryBlockDropZone,
Expand Down Expand Up @@ -307,7 +306,6 @@ addFilter( 'editor.BlockEdit', 'ampStoryEditorBlocks/contextMenuHandler', withCo
addFilter( 'editor.BlockEdit', 'ampStoryEditorBlocks/addEditFeaturedImage', withEditFeaturedImage );
addFilter( 'editor.BlockEdit', 'ampEditorBlocks/addVideoBlockPreview', withCustomVideoBlockEdit, 9 );
addFilter( 'editor.BlockEdit', 'ampEditorBlocks/withSnapLines', withSnapLines );
addFilter( 'editor.PostFeaturedImage', 'ampStoryEditorBlocks/addFeaturedImageNotice', withStoryFeaturedImageNotice );
addFilter( 'editor.BlockListBlock', 'ampStoryEditorBlocks/withActivePageState', withActivePageState );
addFilter( 'editor.BlockListBlock', 'ampStoryEditorBlocks/addWrapperProps', withWrapperProps );
addFilter( 'editor.MediaUpload', 'ampStoryEditorBlocks/addEnforcedFileType', ( InitialMediaUpload ) => withEnforcedFileType( InitialMediaUpload ) );
Expand Down
4 changes: 2 additions & 2 deletions tests/e2e/specs/stories-editor/page.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,15 +113,15 @@ describe( 'Story Page', () => {
} );

it( 'should allow adding opacity', async () => {
await clickButtonByLabel( 'Color: Vivid red' );

const opacitySelector = '.components-range-control__number[aria-label="Opacity"]';
await page.waitForSelector( opacitySelector );

// Set opacity to 15.
await clearInputValue( opacitySelector );
await page.type( opacitySelector, '15' );

await clickButtonByLabel( 'Color: Vivid red' );

await saveDraft();
await page.reload();

Expand Down