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.

Sign up for GitHub

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

Jump to bottom

Enable opacity control (transparency) in blocks that support background color #25421

Closed
wants to merge 6 commits into from
Closed

Enable opacity control (transparency) in blocks that support background color #25421

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fa8bd27
Enable setting alpha in colors, setting it by default to disabled.
Sep 17, 2020
ba3c92d
Ammends previous
Sep 17, 2020
bcf51e4
Returns the rgba serialized
Sep 18, 2020
574f142
Enables transparency for button, paragraph, list, heading,
Sep 18, 2020
3290dbb
Enables changing the opacity of overlay
Oct 2, 2020
2c838bb
Rebase fixes.
May 17, 2021
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
2 changes: 2 additions & 0 deletions packages/block-editor/src/components/colors-gradients/control.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ function ColorGradientControlInner( {
gradients,
disableCustomColors,
disableCustomGradients,
disableAlpha = true,
className,
label,
onColorChange,
Expand Down Expand Up @@ -153,6 +154,7 @@ function ColorGradientControlInner( {
}
{ ...{ colors, disableCustomColors } }
clearable={ clearable }
disableAlpha={ disableAlpha }
/>
) }
{ ( currentTab === 'gradient' || ! canChooseAColor ) && (
Expand Down
1 change: 1 addition & 0 deletions packages/block-editor/src/hooks/color.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,7 @@ export function ColorEdit( props ) {
onGradientChange: hasGradient
? onChangeGradient
: undefined,
disableAlpha: false,
},
]
: [] ),
Expand Down
1 change: 1 addition & 0 deletions packages/block-library/src/button/color-edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ function ColorEdit( props ) {
).color,
gradientValue,
onGradientChange: onChangeGradient,
disableAlpha: false,
},
];
}, [
Expand Down
112 changes: 65 additions & 47 deletions packages/block-library/src/cover/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,10 +387,17 @@ function CoverEdit( {
: {
backgroundImage: gradientValue ? gradientValue : undefined,
} ),
backgroundColor: overlayColor.color,
minHeight: temporaryMinHeight || minHeightWithUnit || undefined,
};

const gradientStyle = {
background: gradientValue,
};

const backgroundStyle = {
backgroundColor: overlayColor.color,
};

const mediaStyle = {
objectPosition:
focalPoint && isImgElement
Expand Down Expand Up @@ -500,49 +507,50 @@ function CoverEdit( {
</PanelRow>
</PanelBody>
) }
<PanelBody title={ __( 'Dimensions' ) }>
<CoverHeightInput
value={ temporaryMinHeight || minHeight }
unit={ minHeightUnit }
onChange={ ( newMinHeight ) =>
setAttributes( { minHeight: newMinHeight } )
}
onUnitChange={ ( nextUnit ) =>
setAttributes( {
minHeightUnit: nextUnit,
} )
}
/>
</PanelBody>
<PanelColorGradientSettings
title={ __( 'Overlay' ) }
initialOpen={ true }
settings={ [
{
colorValue: overlayColor.color,
gradientValue,
onColorChange: setOverlayColor,
onGradientChange: setGradient,
label: __( 'Color' ),
},
] }
>
{ !! url && (
<RangeControl
label={ __( 'Opacity' ) }
value={ dimRatio }
onChange={ ( newDimRation ) =>
setAttributes( {
dimRatio: newDimRation,
} )
}
min={ 0 }
max={ 100 }
step={ 10 }
required
/>
) }
</PanelColorGradientSettings>
{ hasBackground && (
<>
<PanelBody title={ __( 'Dimensions' ) }>
<CoverHeightInput
value={ temporaryMinHeight || minHeight }
unit={ minHeightUnit }
onChange={ ( newMinHeight ) =>
setAttributes( { minHeight: newMinHeight } )
}
onUnitChange={ ( nextUnit ) => {
setAttributes( {
minHeightUnit: nextUnit,
} );
} }
/>
</PanelBody>
<PanelColorGradientSettings
title={ __( 'Overlay' ) }
initialOpen={ true }
settings={ [
{
colorValue: overlayColor.color,
gradientValue,
onColorChange: setOverlayColor,
onGradientChange: setGradient,
label: __( 'Color' ),
},
] }
>
<RangeControl
label={ __( 'Opacity' ) }
value={ dimRatio }
onChange={ ( newDimRation ) =>
setAttributes( {
dimRatio: newDimRation,
} )
}
min={ 0 }
max={ 100 }
required
/>
</PanelColorGradientSettings>
</>
) }
</InspectorControls>
</>
);
Expand Down Expand Up @@ -634,14 +642,24 @@ function CoverEdit( {
} }
showHandle={ isSelected }
/>
{ url && gradientValue && dimRatio !== 0 && (
{ overlayColor.color && ! gradientValue && dimRatio !== 0 && (
<span
aria-hidden="true"
className={ classnames(
'wp-block-cover__background',
classes
) }
style={ backgroundStyle }
/>
) }
{ gradientValue && ! overlayColor.color && dimRatio !== 0 && (
<span
Copy link
Member

Choose a reason for hiding this comment

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

We are adding this span element to the edit function but not to the save markup that the block produces and is shown on the frontend. Is the span also required on the frontend? If yes, I guess we may need to add it to the save function, but that is complex:

  • It will require CSS that works with the new block markup and with the old one.
  • It will require block deprecations.

Is there any way we can accomplish the same without requiring the additional span?

A possible way would to when there is an opacity value set, and there's a color change, we would add the alpha value to the color attribute even though in the color picker alpha does not appear. It is a little bit hacky and makes one attribute depend on the other, but that's only on editing on the frontend; no changes are required. For the gradients, I guess our gradients library would need to expose some function that allows changing every stop of the gradient to a specific alpha. This solution is also a little bit complex; the main advantage is that the dom we produce would be simpler as we would not require a dom element just for a color background.

In the future, we will probably move the image background to a dom element too. To take advantage of the server-side processing, WordPress does for images. If we follow that refactor, I wonder if it would make sense to join this work and that image change to avoid the need to add deprecations two times and the need to have our CSS code support more two different versions of the markup. In that case, I think it may make sense to not do the change to cover yet, and then do a wide refactor to the cover block with all these changes that are pending.

What do you think @jasmussen?

Copy link
Contributor

Choose a reason for hiding this comment

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

In the future, we will probably move the image background to a dom element too. To take advantage of the server-side processing, WordPress does for images. If we follow that refactor, I wonder if it would make sense to join this work and that image change to avoid the need to add deprecations two times and the need to have our CSS code support more two different versions of the markup. In that case, I think it may make sense to not do the change to cover yet, and then do a wide refactor to the cover block with all these changes that are pending.

This sounds very reasonable. As far as I'm aware, there are many pros to moving to a regular image, including around how srcsets work. The primary challenge, as I perceive it, is to refactor how the fixed position works, where currently it's a simple background-position change, whereas were it an image, it'd probably need to combine some position: fixed; with overflow: hidden; on a parent.

CC: @ajlende as I believe you've been involved in related work.

In the mean time, I would agree that it'd be best to find a way to avoid block deprecations. I have some thing to look into elsewhere today, but when I have a moment, I'm going to try and dive into the PR and see how I may be able to help.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jorgefilipecosta The work for moving the cover block background image is in #25171. It's just kinda sitting there waiting for a +1.

That PR doesn't include changes for the fixed or "parallax" mode that Joen is referring to, so if we're trying to limit the number of deprecations, we'd also probably want to include that change. I can add it to #25171 or we can open another PR to review it separately and join the three together before merging them all.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ajlende just to be sure I'm understanding you correctly, #25171 removes the "parallax" feature from the cover block, correct? I guess that makes sense insofar as now that I think about it, it's really tricky to replicate with an img, because position: fixed; puts it outside of the containers stacking order, so it can't be "cropped" by the cover container.

But that's still a big bummer. Is there some basic parallax JavaScript that we can use to replicate the parallax effect, without deprecating it?

Copy link
Contributor

Choose a reason for hiding this comment

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

#25171 didn't touch the parallax feature—choosing parallax sets the background-image property as before and is the only option that doesn't use srcset. That's what I was referring to when I was talking about opening a third PR—to see if it's worth trying to find a JS solution which will allow both srcset and the parallax effect.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh that's smart! That seems totally fine. Thank you for the clarification.

aria-hidden="true"
className={ classnames(
'wp-block-cover__gradient-background',
gradientClass
classes
) }
style={ { backgroundImage: gradientValue } }
style={ gradientStyle }
/>
) }
{ url && isImageBackground && isImgElement && (
Expand Down
18 changes: 10 additions & 8 deletions packages/block-library/src/cover/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
padding: 1em;
// This block has customizable padding, border-box makes that more predictable.
box-sizing: border-box;
background-color: transparent;
Copy link
Member

Choose a reason for hiding this comment

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

These style changes are causing problems for existing cover blocks.
In all cases where we have an image or video background and an overlay color or gradient. With these changes, the overlay color and gradients stop appearing on the frontend of the websites.

To reproduce one can add some cover blocks select image, video backgrounds, and overlay color /gradients save the post and see it on the front end. The overlays will not be there.


&.has-parallax {
background-attachment: fixed;
Expand Down Expand Up @@ -47,20 +48,15 @@
background-color: $black;
}

&.has-background-dim::before {
content: "";
background-color: inherit;
}

&.has-background-dim:not(.has-background-gradient)::before,
.wp-block-cover__gradient-background {
.has-background-dim:not(.has-background-gradient),
.wp-block-cover__gradient-background,
.wp-block-cover__background {
position: absolute;
top: 0;
left: 0;
bottom: 0;
right: 0;
z-index: z-index(".wp-block-cover.has-background-dim::before");
opacity: 0.5;
}


Expand All @@ -71,6 +67,12 @@
opacity: $i * 0.1;
}
}
.has-background-dim.has-background-dim-#{ $i * 10 } {
&.wp-block-cover__background,
&.wp-block-cover__gradient-background {
opacity: $i * 0.1;
}
}
}

// Apply max-width to floated items that have no intrinsic width
Expand Down
10 changes: 8 additions & 2 deletions packages/components/src/color-palette/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,14 @@ import { useCallback, useMemo } from '@wordpress/element';
*/
import ColorPicker from '../color-picker';
import CircularOptionPicker from '../circular-option-picker';
import { rgba } from '../utils';

export default function ColorPalette( {
clearable = true,
className,
colors,
disableCustomColors = false,
disableAlpha = true,
onChange,
value,
} ) {
Expand Down Expand Up @@ -61,8 +63,12 @@ export default function ColorPalette( {
const renderCustomColorPicker = () => (
<ColorPicker
color={ value }
onChangeComplete={ ( color ) => onChange( color.hex ) }
disableAlpha
onChangeComplete={ ( color ) =>
onChange(
disableAlpha ? color.hex : rgba( color.hex, color.rgb.a )
Copy link
Member

Choose a reason for hiding this comment

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

I think when alpha is not set we should store the colors in hexadecimal, even if picking alpha is enabled. The onChange condition should not be based on alpha being enabled or not but on alpha being set or not.

)
}
disableAlpha={ disableAlpha }
/>
);

Expand Down