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

Add border support to site logo #48354

Open
wants to merge 68 commits into
base: trunk
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
68 commits
Select commit Hold shift + click to select a range
3cb7840
Add border support to site logo
carolinan Feb 23, 2023
41cdda1
Update index.php
carolinan Feb 24, 2023
1ea0a5f
Merge branch 'trunk' into add/site-logo-border
carolinan Mar 27, 2023
279019b
Merge branch 'trunk' into add/site-logo-border
carolinan Apr 19, 2023
47125e1
Merge branch 'trunk' into add/site-logo-border
carolinan Apr 21, 2023
53892f4
Merge branch 'trunk' into add/site-logo-border
carolinan May 11, 2023
b91090f
Merge branch 'trunk' into add/site-logo-border
carolinan Jun 14, 2023
860ad11
Merge branch 'trunk' into add/site-logo-border
carolinan Jun 19, 2023
dbf9900
Merge branch 'trunk' into add/site-logo-border
carolinan Jun 26, 2023
6d506b9
Merge branch 'trunk' into add/site-logo-border
carolinan Jun 27, 2023
5602da2
Add a check for the border CSS classes
carolinan Jun 27, 2023
177b072
Add the custom border to the ImageEditor / cropper
carolinan Jun 27, 2023
d30eccd
Merge branch 'trunk' into add/site-logo-border
carolinan Jul 11, 2023
1b00379
Merge branch 'trunk' into add/site-logo-border
carolinan Jul 21, 2023
06b939d
Merge branch 'trunk' into add/site-logo-border
carolinan Aug 7, 2023
501fb91
try height: auto, remove border when placeholder is selected
carolinan Aug 8, 2023
882acf6
try to fix linked image width
carolinan Aug 8, 2023
0c5ab73
Move the CSS for the linked image width
carolinan Aug 8, 2023
8a74ce8
Merge branch 'trunk' into add/site-logo-border
carolinan Aug 11, 2023
97b3a40
Merge branch 'trunk' into add/site-logo-border
carolinan Sep 26, 2023
c340f86
CS: Rename function name
carolinan Sep 26, 2023
09a0c96
Merge branch 'trunk' into add/site-logo-border
carolinan Oct 13, 2023
549e075
Merge branch 'trunk' into add/site-logo-border
carolinan Oct 25, 2023
2249153
Fix editing tool border color by moving the <a> element
carolinan Oct 25, 2023
0df97a5
Merge branch 'trunk' into add/site-logo-border
carolinan Nov 15, 2023
c1d747d
Fix the JavaScript error when the link option is toggled off
carolinan Nov 15, 2023
ab6c5d4
Update the <a> image wrapper and editor styles. Add 'has-custom-boder…
carolinan Nov 20, 2023
4f19a03
Merge branch 'trunk' into add/site-logo-border
carolinan Nov 23, 2023
038906b
Add display:table to centered logos
carolinan Nov 23, 2023
5e438e3
Merge branch 'trunk' into add/site-logo-border
carolinan Nov 24, 2023
a76a4ac
Code cleanup based on review
carolinan Nov 24, 2023
6bb7388
Merge branch 'trunk' into add/site-logo-border
carolinan Nov 24, 2023
6552759
Merge branch 'trunk' into add/site-logo-border
carolinan Jan 3, 2024
db034b7
Merge branch 'trunk' into add/site-logo-border
carolinan Jan 9, 2024
122e4a7
Update editor.scss
carolinan Jan 9, 2024
2e987e5
Try to fix the height difference that occurs when the parent element …
carolinan Jan 10, 2024
17d4c72
Merge branch 'trunk' into add/site-logo-border
carolinan Jan 18, 2024
cf4af0d
Remove undefined check for the borderProps.className
carolinan Jan 18, 2024
45702b0
Remove unwanted and duplicate CSS in editor.scss
carolinan Jan 18, 2024
3b94538
Merge branch 'trunk' into add/site-logo-border
carolinan Jan 25, 2024
9c698f5
Simplify the conditions and assignments in block_core_site_logo_get_b…
carolinan Jan 25, 2024
43eabd8
Move the border selector from supports to selectors.
carolinan Jan 25, 2024
3d8c582
Merge branch 'trunk' into add/site-logo-border
carolinan Feb 3, 2024
90cb781
Update style.scss
carolinan Feb 3, 2024
551db7a
Merge branch 'trunk' into add/site-logo-border
carolinan Feb 14, 2024
0d62516
Merge branch 'trunk' into add/site-logo-border
carolinan Feb 22, 2024
c879a2e
Merge branch 'trunk' into add/site-logo-border
carolinan Mar 20, 2024
cfb275a
Fix coding standard issues
carolinan Mar 20, 2024
33bd8af
Merge branch 'trunk' into add/site-logo-border
carolinan Mar 21, 2024
357bad8
Merge branch 'trunk' into add/site-logo-border
carolinan Apr 4, 2024
bc5a7ef
Revert the change to .wp-block-site-logo.aligncenter.
carolinan Apr 4, 2024
635476c
Try to fix merge conflicts, attempt 1
carolinan Apr 24, 2024
97fa100
Merge branch 'trunk' into add/site-logo-border
carolinan Apr 24, 2024
9fbe66f
Merge branch 'trunk' into add/site-logo-border
carolinan May 7, 2024
7eadd08
Fix issues after the merge conflict
carolinan May 7, 2024
8f91248
Merge branch 'trunk' into add/site-logo-border
carolinan May 31, 2024
8b08a4f
Merge branch 'trunk' into add/site-logo-border
carolinan May 31, 2024
3aa4a50
Merge branch 'trunk' into add/site-logo-border
carolinan Jul 1, 2024
7aeadef
Merge branch 'trunk' into add/site-logo-border
carolinan Jul 10, 2024
d2374e8
Merge branch 'trunk' into add/site-logo-border
carolinan Jul 31, 2024
b9008b8
Remove experimental default controls
carolinan Jul 31, 2024
b4eb312
Border attributes: Use add_class instead of set_attributes
carolinan Jul 31, 2024
07117f1
Try to remove the CSS classes from the ImageWrapper to test if anythi…
carolinan Jul 31, 2024
46c352b
Show block border styles when the placeholder is selected.
carolinan Jul 31, 2024
31f50cf
WIP: update CSS reduce specificity
carolinan Jul 31, 2024
12cdcf0
Add the size limitation back
carolinan Aug 1, 2024
95f264f
Try resetting the box-sizing on the placeholder.
carolinan Aug 1, 2024
1e7f619
Merge branch 'trunk' into add/site-logo-border
carolinan Aug 12, 2024
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
10 changes: 10 additions & 0 deletions packages/block-library/src/site-logo/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@
"html": false,
"align": true,
"alignWide": false,
"__experimentalBorder": {
"color": true,
"radius": true,
"width": true,
"style": true,
"__experimentalSkipSerialization": true
},
"color": {
"__experimentalDuotone": "img, .components-placeholder__illustration, .components-placeholder::before",
"text": false,
Expand All @@ -50,6 +57,9 @@
"clientNavigation": true
}
},
"selectors": {
"border": ".wp-block-site-logo img, .wp-block-site-logo .components-placeholder, .wp-block-site-logo .wp-block-image__crop-area"
},
"styles": [
{
"name": "default",
Expand Down
120 changes: 78 additions & 42 deletions packages/block-library/src/site-logo/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import {
useBlockProps,
store as blockEditorStore,
__experimentalImageEditor as ImageEditor,
__experimentalUseBorderProps as useBorderProps,
} from '@wordpress/block-editor';
import { useSelect, useDispatch } from '@wordpress/data';
import { store as coreStore } from '@wordpress/core-data';
Expand All @@ -54,9 +55,37 @@ import { MIN_SIZE } from '../image/constants';
const ALLOWED_MEDIA_TYPES = [ 'image' ];
const ACCEPT_MEDIA_STRING = 'image/*';

// If the logo is linked, wrap in an <a /> tag to trigger any inherited link element styles.
const ImageWrapper = ( { isLink, href, title, children } ) => {
if ( ! isLink ) {
return children;
}
return (
<a
href={ href }
className="custom-logo-link"
title={ title }
rel="home"
onClick={ ( event ) => event.preventDefault() }
aria-disabled
style={ {
// When the site logo block is linked,
// it's wrapped with a disabled <a /> tag.
// Restore cursor style so it doesn't appear 'clickable'
// and remove pointer events. Safari needs the display property.
pointerEvents: 'none',
cursor: 'default',
display: 'inline',
} }
>
{ children }
</a>
);
};

const SiteLogo = ( {
alt,
attributes: { align, width, height, isLink, linkTarget, shouldSyncIcon },
attributes,
isSelected,
setAttributes,
setLogo,
Expand All @@ -67,12 +96,16 @@ const SiteLogo = ( {
setIcon,
canUserEdit,
} ) => {
const { align, width, height, isLink, linkTarget, shouldSyncIcon } =
attributes;
const isLargeViewport = useViewportMatch( 'medium' );
const isWideAligned = [ 'wide', 'full' ].includes( align );
const isResizable = ! isWideAligned && isLargeViewport;
const [ { naturalWidth, naturalHeight }, setNaturalSize ] = useState( {} );
const [ isEditingImage, setIsEditingImage ] = useState( false );
const { toggleSelection } = useDispatch( blockEditorStore );
const borderProps = useBorderProps( attributes );

const { imageEditing, maxWidth, title } = useSelect( ( select ) => {
const settings = select( blockEditorStore ).getSettings();
const siteEntities = select( coreStore ).getEntityRecord(
Expand All @@ -93,7 +126,7 @@ const SiteLogo = ( {
if ( shouldSyncIcon && logoId !== iconId ) {
setAttributes( { shouldSyncIcon: false } );
}
}, [] );
}, [ iconId, logoId, setAttributes, shouldSyncIcon ] );

useEffect( () => {
if ( ! isSelected ) {
Expand All @@ -112,7 +145,7 @@ const SiteLogo = ( {
const img = (
<>
<img
className="custom-logo"
className={ clsx( 'custom-logo', borderProps.className ) }
src={ logoUrl }
alt={ alt }
onLoad={ ( event ) => {
Expand All @@ -121,33 +154,24 @@ const SiteLogo = ( {
naturalHeight: event.target.naturalHeight,
} );
} }
style={ borderProps.style }
/>
{ isBlobURL( logoUrl ) && <Spinner /> }
</>
);

let imgWrapper = img;

// Disable reason: Image itself is not meant to be interactive, but
// should direct focus to block.
if ( isLink ) {
imgWrapper = (
/* eslint-disable jsx-a11y/no-noninteractive-element-interactions, jsx-a11y/click-events-have-key-events */
<a
href={ siteUrl }
className="custom-logo-link"
rel="home"
title={ title }
onClick={ ( event ) => event.preventDefault() }
>
{ img }
</a>
/* eslint-enable jsx-a11y/no-noninteractive-element-interactions, jsx-a11y/click-events-have-key-events */
);
}

if ( ! isResizable || ! naturalWidth || ! naturalHeight ) {
return <div style={ { width, height } }>{ imgWrapper }</div>;
return (
<div style={ { width, height } }>
<ImageWrapper
isLink={ isLink }
href={ siteUrl }
title={ title }
>
{ img }
</ImageWrapper>
</div>
);
}

// Set the default width to a responsible size.
Expand Down Expand Up @@ -207,20 +231,23 @@ const SiteLogo = ( {

const imgEdit =
canEditImage && isEditingImage ? (
<ImageEditor
id={ logoId }
url={ logoUrl }
width={ currentWidth }
height={ currentHeight }
naturalHeight={ naturalHeight }
naturalWidth={ naturalWidth }
onSaveImage={ ( imageAttributes ) => {
setLogo( imageAttributes.id );
} }
onFinishEditing={ () => {
setIsEditingImage( false );
} }
/>
<ImageWrapper isLink={ isLink } href={ siteUrl } title={ title }>
<ImageEditor
id={ logoId }
url={ logoUrl }
width={ currentWidth }
height={ currentHeight }
naturalHeight={ naturalHeight }
naturalWidth={ naturalWidth }
onSaveImage={ ( imageAttributes ) => {
setLogo( imageAttributes.id );
} }
onFinishEditing={ () => {
setIsEditingImage( false );
} }
borderProps={ borderProps }
/>
</ImageWrapper>
) : (
<ResizableBox
size={ {
Expand Down Expand Up @@ -248,7 +275,13 @@ const SiteLogo = ( {
} );
} }
>
{ imgWrapper }
<ImageWrapper
isLink={ isLink }
href={ siteUrl }
title={ title }
>
{ img }
</ImageWrapper>
</ResizableBox>
);

Expand Down Expand Up @@ -407,6 +440,7 @@ export default function LogoEdit( {
isSelected,
} ) {
const { width, shouldSyncIcon } = attributes;
const borderProps = useBorderProps( attributes );
const {
siteLogoId,
canUserEdit,
Expand Down Expand Up @@ -590,9 +624,7 @@ export default function LogoEdit( {
className={ placeholderClassName }
preview={ logoImage }
withIllustration
style={ {
width,
} }
style={ ( width, borderProps.style ) }
>
{ content }
</Placeholder>
Expand All @@ -602,6 +634,10 @@ export default function LogoEdit( {
const classes = clsx( className, {
'is-default-size': ! width,
'is-transient': temporaryURL,
'has-custom-border':
!! borderProps.className ||
( borderProps.style &&
Object.keys( borderProps.style ).length > 0 ),
} );

const blockProps = useBlockProps( { className: classes } );
Expand Down
44 changes: 36 additions & 8 deletions packages/block-library/src/site-logo/editor.scss
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,6 @@
}

.wp-block-site-logo {
// Make the block selectable.
a {
pointer-events: none;
}

.custom-logo-link {
cursor: inherit;

Expand All @@ -21,8 +16,10 @@

img {
display: block;
height: auto;
max-width: 100%;
}

.wp-block-image__crop-area {
pointer-events: auto;
}

&.is-transient {
Expand Down Expand Up @@ -52,19 +49,20 @@
width: 60px;
}
Copy link
Contributor Author

@carolinan carolinan Jul 31, 2024

Choose a reason for hiding this comment

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

If I don't remove the height and width, the upload button is misaligned when there is a border.
In this screenshot, a border width of 10px is added in global styles:
logo-placeholder

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure what this CSS is for, was 60px chosen because the default size of the logo is 120?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I found the ticket #48218
I am not sure how to resolve it without letting it fall back to height: 100% and width:100% though! @richtabor

Copy link
Member

Choose a reason for hiding this comment

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

I am not sure what this CSS is for, was 60px chosen because the default size of the logo is 120?

Yes, per https://github.com/WordPress/gutenberg/pull/48218—120x120 is too big for a logo placeholder, but 120px is the max initial width for when a logo is uploaded.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a tricky one 🤔

While in the placeholder state the layout shift when adding a border sticks out. When not in the placeholder state the block has border-box set for the box-sizing. Adding border-box for the placeholder state exacerbates the problem with scrollbars etc.

I'm not sure there is a bulletproof solution but maybe there is something we can do to smooth out the experience a bit for the less extreme use cases.

If we avoid applying the custom border on the placeholder directly and instead apply it to a new pseudo element. We can position the bordered pseudo element over the inner contents. It will start to crop the underlying button as the border increases but that seems better than scrollbars.

Hacking around in dev tools this is what I got:

Screen.Recording.2024-08-08.at.8.05.37.PM.mp4


/**
// Inherit radius.
> div, // A 60px width div shown only in the editor on mobile.
.components-resizable-box__container {
border-radius: inherit;
}
Copy link
Contributor Author

@carolinan carolinan Jul 31, 2024

Choose a reason for hiding this comment

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

The border-radius:inherit on the >div here is overriding the border radius from the global styles.
I did not understand the code comment about the div only being shown in the editor on mobile, or how to test it.

*/

// Style the placeholder.
.components-placeholder {
display: flex;
justify-content: center;
align-items: center;
padding: 0;
border-radius: inherit;

// Provide a minimum size for the placeholder, for when the logo is resized.
// @todo resizing is currently only possible by adding an image, resizing,
Expand Down Expand Up @@ -108,6 +106,18 @@
}
}

// Reduced specificity for the placeholder border radius,
// so that border-radius from global styles can be applied.
:root :where(.wp-block-site-logo .components-placeholder) {
border-radius: inherit;
}

Copy link
Contributor Author

@carolinan carolinan Jul 31, 2024

Choose a reason for hiding this comment

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

Even with the border-radius:inherit

  1. Commented out on line 58
  2. Moved from line 67 to 114 with reduced specificity

the placeholder component's own CSS still overrides the border radius from the global styles.

// Reset the box-sizing for the placeholder to prevent the border
// from covering the logo upload button.
.wp-block-site-logo .components-placeholder.components-placeholder {
box-sizing: initial;
}

.block-library-site-logo__inspector-upload-container {
position: relative;
// Since there is no option to skip rendering the drag'n'drop icon in drop
Expand Down Expand Up @@ -165,3 +175,21 @@
height: $grid-unit-50;
}
}

// This is necessary for the editor resize handles to accurately work on a non-floated, non-resized logo.
.wp-block-site-logo .components-resizable-box__container {
// Using "display: table" because:
// - it visually hides empty white space in between elements
// - it allows the element to be as wide as its contents (instead of 100% width, as it would be with `display: block`)
display: table;
img {
display: block;
width: inherit;
}
}

.wp-block-site-logo.has-custom-border {
.wp-block-image__crop-area {
box-sizing: border-box;
}
}
59 changes: 59 additions & 0 deletions packages/block-library/src/site-logo/index.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,22 @@ function render_block_core_site_logo( $attributes ) {
}

$classnames = array();

$border_attributes = block_core_site_logo_get_border_attributes( $attributes );
if ( $border_attributes ) {
$classnames[] = 'has-custom-border';

$processor = new WP_HTML_Tag_Processor( $custom_logo );
$processor->next_tag( 'img' );
if ( ! empty( $border_attributes['class'] ) ) {
$processor->add_class( $border_attributes['class'] );
}
if ( ! empty( $border_attributes['style'] ) ) {
$processor->set_attribute( 'style', $border_attributes['style'] );
}
$custom_logo = $processor->get_updated_html();
}

if ( empty( $attributes['width'] ) ) {
$classnames[] = 'is-default-size';
}
Expand All @@ -60,6 +76,49 @@ function render_block_core_site_logo( $attributes ) {
return $html;
}

/**
* Generates class names and styles to apply the border support styles for
* the site logo block.
*
* @since 6.7.0
*
* @param array $attributes The block attributes.
* @return array The border-related classnames and styles for the block.
*/
function block_core_site_logo_get_border_attributes( $attributes ) {
carolinan marked this conversation as resolved.
Show resolved Hide resolved
$sides = array( 'top', 'right', 'bottom', 'left' );
$border_styles = array(
'radius' => $attributes['style']['border']['radius'] ?? null,
'style' => $attributes['style']['border']['style'] ?? null,
'width' => $attributes['style']['border']['width'] ?? null,
);

// Border color.
$preset_color = array_key_exists( 'borderColor', $attributes ) ? "var:preset|color|{$attributes['borderColor']}" : null;
$custom_color = $attributes['style']['border']['color'] ?? null;
$border_styles['color'] = $preset_color ?? $custom_color;

// Individual border styles e.g. top, left etc.
foreach ( $sides as $side ) {
$border = $attributes['style']['border'][ $side ] ?? null;
$border_styles[ $side ] = array(
'color' => $border['color'] ?? null,
'style' => $border['style'] ?? null,
'width' => $border['width'] ?? null,
);
}

$styles = wp_style_engine_get_styles( array( 'border' => $border_styles ) );
$attributes = array();
if ( ! empty( $styles['classnames'] ) ) {
$attributes['class'] = $styles['classnames'];
}
if ( ! empty( $styles['css'] ) ) {
$attributes['style'] = $styles['css'];
}
return $attributes;
}

/**
* Register a core site setting for a site logo
*
Expand Down
Loading
Loading