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

Feature: Added InnerBlock Support for Download Button & Add Support for download attribute #68510

Open
wants to merge 3 commits into
base: trunk
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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: 1 addition & 1 deletion docs/reference-guides/core-blocks.md
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ Add a link to a downloadable file. ([Source](https://github.com/WordPress/gutenb
- **Name:** core/file
- **Category:** media
- **Supports:** align, anchor, color (background, gradients, link, ~~text~~), interactivity, spacing (margin, padding)
- **Attributes:** blob, displayPreview, downloadButtonText, fileId, fileName, href, id, previewHeight, showDownloadButton, textLinkHref, textLinkTarget
- **Attributes:** blob, displayPreview, fileId, fileName, href, id, previewHeight, showDownloadButton, textLinkHref, textLinkTarget

## Footnotes

Expand Down
6 changes: 0 additions & 6 deletions packages/block-library/src/file/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,6 @@
"type": "boolean",
"default": true
},
"downloadButtonText": {
"type": "rich-text",
"source": "rich-text",
"selector": "a[download]",
"role": "content"
},
"displayPreview": {
"type": "boolean"
},
Expand Down
73 changes: 32 additions & 41 deletions packages/block-library/src/file/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ import {
RichText,
useBlockProps,
store as blockEditorStore,
__experimentalGetElementClassName,
InnerBlocks,
} from '@wordpress/block-editor';
import { useEffect, useState } from '@wordpress/element';
import { useState } from '@wordpress/element';
import { useCopyToClipboard } from '@wordpress/compose';
import { __, _x } from '@wordpress/i18n';
import { __ } from '@wordpress/i18n';
import { file as icon } from '@wordpress/icons';
import { store as coreStore } from '@wordpress/core-data';
import { store as noticesStore } from '@wordpress/notices';
Expand Down Expand Up @@ -69,7 +69,6 @@ function FileEdit( { attributes, isSelected, setAttributes, clientId } ) {
textLinkHref,
textLinkTarget,
showDownloadButton,
downloadButtonText,
displayPreview,
previewHeight,
} = attributes;
Expand All @@ -85,27 +84,14 @@ function FileEdit( { attributes, isSelected, setAttributes, clientId } ) {
);

const { createErrorNotice } = useDispatch( noticesStore );
const { toggleSelection, __unstableMarkNextChangeAsNotPersistent } =
useDispatch( blockEditorStore );
const { toggleSelection } = useDispatch( blockEditorStore );

useUploadMediaFromBlobURL( {
url: temporaryURL,
onChange: onSelectFile,
onError: onUploadError,
} );

// Note: Handle setting a default value for `downloadButtonText` via HTML API
// when it supports replacing text content for HTML tags.
useEffect( () => {
if ( RichText.isEmpty( downloadButtonText ) ) {
__unstableMarkNextChangeAsNotPersistent();
setAttributes( {
downloadButtonText: _x( 'Download', 'button label' ),
} );
}
// This effect should only run on mount.
}, [] );

function onSelectFile( newMedia ) {
if ( ! newMedia || ! newMedia.url ) {
// Reset attributes.
Expand Down Expand Up @@ -302,29 +288,34 @@ function FileEdit( { attributes, isSelected, setAttributes, clientId } ) {
href={ textLinkHref }
/>
{ showDownloadButton && (
<div className="wp-block-file__button-richtext-wrapper">
{ /* Using RichText here instead of PlainText so that it can be styled like a button. */ }
<RichText
identifier="downloadButtonText"
tagName="div" // Must be block-level or else cursor disappears.
aria-label={ __( 'Download button text' ) }
className={ clsx(
'wp-block-file__button',
__experimentalGetElementClassName(
'button'
)
) }
value={ downloadButtonText }
withoutInteractiveFormatting
placeholder={ __( 'Add text…' ) }
onChange={ ( text ) =>
setAttributes( {
downloadButtonText:
removeAnchorTag( text ),
} )
}
/>
</div>
<InnerBlocks
Copy link
Member

Choose a reason for hiding this comment

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

In almost all cases, you want to use useInnerBlocksProps rather than <InnerBlocks /> these days because that allows you to match the markup between the editor and the frontend (you don't get additional div elements injected in the editor)

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 see, thanks for pointing that out. I'll look into updating it to use useInnerBlocksProps!

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 have updated the code to reflect the changes with useInnerBlocksProps. Thanks

allowedBlocks={ [ 'core/buttons' ] }
template={ [
[
'core/buttons',
{},
[
[
'core/button',
{
text: __( 'Download' ),
lock: {
remove: true,
move: true,
},
url: href || temporaryURL,
download: true,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

download attribute is missing, should we have it added to button component or should we go with a work around here?

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 went ahead with implementing the download attribute on button block for now, I think there is a need for discussion if alternative exists but I feel implementing download attribute is better way to handle this.

ariaLabel: __(
'Download button text'
),
},
],
],
],
] }
templateLock="all"
renderAppender={ false }
/>
) }
</div>
</div>
Expand Down
15 changes: 9 additions & 6 deletions packages/block-library/src/file/editor.scss
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,15 @@
}

.wp-block-file__content-wrapper {
flex-grow: 1;
display: flex;
align-items: center;
gap: 1em;
flex-wrap: wrap;
width: 100%;

.block-editor-inner-blocks {
flex: 1;
}
}

a {
Expand All @@ -43,9 +51,4 @@
display: inline-block;
}
}

.wp-block-file__button-richtext-wrapper {
display: inline-block;
margin-left: 0.75em;
}
}
26 changes: 4 additions & 22 deletions packages/block-library/src/file/save.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,7 @@
/**
* External dependencies
*/
import clsx from 'clsx';

/**
* WordPress dependencies
*/
import {
RichText,
useBlockProps,
__experimentalGetElementClassName,
} from '@wordpress/block-editor';
import { RichText, useBlockProps, InnerBlocks } from '@wordpress/block-editor';

export default function save( { attributes } ) {
const {
Expand All @@ -20,7 +11,6 @@ export default function save( { attributes } ) {
textLinkHref,
textLinkTarget,
showDownloadButton,
downloadButtonText,
displayPreview,
previewHeight,
} = attributes;
Expand Down Expand Up @@ -67,17 +57,9 @@ export default function save( { attributes } ) {
</a>
) }
{ showDownloadButton && (
<a
href={ href }
className={ clsx(
'wp-block-file__button',
__experimentalGetElementClassName( 'button' )
) }
download
aria-describedby={ describedById }
>
<RichText.Content value={ downloadButtonText } />
</a>
<div className="wp-block-file__button_wrapper">
<InnerBlocks.Content />
</div>
) }
</div>
)
Expand Down
42 changes: 25 additions & 17 deletions packages/block-library/src/file/style.scss
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
.wp-block-file {
// This block has customizable padding, border-box makes that more predictable.
box-sizing: border-box;
display: flex;
flex-wrap: wrap;
align-items: center;
gap: 1em;

&:not(.wp-element-button) {
font-size: 0.8em;
Expand All @@ -18,6 +22,10 @@
* + .wp-block-file__button {
margin-left: 0.75em;
}

.wp-block-file__button_wrapper {
flex: 1;
}
}

// Lowest specificity to avoid overriding layout styles.
Expand All @@ -30,20 +38,20 @@
}

//This needs a low specificity so it won't override the rules from the button element if defined in theme.json.
:where(.wp-block-file__button) {
border-radius: 2em;
padding: 0.5em 1em;
display: inline-block;

&:is(a) {
&:hover,
&:visited,
&:focus,
&:active {
box-shadow: none;
color: $white;
opacity: 0.85;
text-decoration: none;
}
}
}
// :where(.wp-block-file__button) {
// border-radius: 2em;
// padding: 0.5em 1em;
// display: inline-block;

// &:is(a) {
// &:hover,
// &:visited,
// &:focus,
// &:active {
// box-shadow: none;
// color: $white;
// opacity: 0.85;
// text-decoration: none;
// }
// }
// }
Loading