-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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 Site Title block and required functionality. #17207
Changes from all commits
614b3a9
e5d598c
8e028b3
edaf27f
eccad2c
054e6c9
3b14005
3ac3b1b
9d3890e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
{ | ||
"name": "core/site-title", | ||
"category": "layout" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { | ||
useEntityProp, | ||
__experimentalUseEntitySaving, | ||
} from '@wordpress/core-data'; | ||
import { Button } from '@wordpress/components'; | ||
import { __ } from '@wordpress/i18n'; | ||
import { RichText } from '@wordpress/block-editor'; | ||
|
||
export default function SiteTitleEdit() { | ||
const [ title, setTitle ] = useEntityProp( 'root', 'site', 'title' ); | ||
const [ isDirty, isSaving, save ] = __experimentalUseEntitySaving( | ||
'root', | ||
'site', | ||
'title' | ||
); | ||
return ( | ||
<> | ||
<Button | ||
isPrimary | ||
className="wp-block-site-title__save-button" | ||
disabled={ ! isDirty || ! title } | ||
isBusy={ isSaving } | ||
onClick={ save } | ||
> | ||
{ __( 'Update' ) } | ||
</Button> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can probably remove that button once we land the other PR (multi-entity saving) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep! |
||
<RichText | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about adding There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's also There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AFAIK There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I'm adding it now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel like if it doesn't support HTML at all, PlainText is the way to go. Even if we disallow formats from a RichText, it's still a RichText and someone can tweak the HTML and pass it and it will render. We should seek to fix PlainText issues instead IMO. How can we make it easier to style... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While I can see the reasons against using
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are all good reasons to improve PlainText and not use RichText to me. 1- If we one a single line text, we can add a prop for that and use an input. Worst-case scenario we could refactor PlainText to use a contenteditable but we shouldn't be using RichText as it's mean for RichText (formatted html) and we shouldn't try to hack its purpose. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I can give it a try, if y'all don't mind. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please do, that's great 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I gave this a try and have some doubts.
Though, As of now, I've got some good enough ways to prevent line breaks (maybe replacing them with an
This is where I'm stuck.
This was actually easier than expected: the
In the end, I think we are back to this. I'm not being dismissive, but I'm unsure how to proceed. |
||
tagName="h1" | ||
placeholder={ __( 'Site Title' ) } | ||
value={ title } | ||
onChange={ setTitle } | ||
allowedFormats={ [] } | ||
/> | ||
</> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
.wp-block-site-title__save-button { | ||
position: absolute; | ||
right: 0; | ||
top: 0; | ||
z-index: z-index(".wp-block-site-title__save-button"); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { SVG, Path, Circle } from '@wordpress/components'; | ||
|
||
export default ( | ||
<SVG xmlns="https://www.w3.org/2000/svg" viewBox="0 0 24 24"> | ||
<Path fill="none" d="M0 0h24v24H0V0z" /> | ||
<Path d="M12 2C8.13 2 5 5.13 5 9c0 5.25 7 13 7 13s7-7.75 7-13c0-3.87-3.13-7-7-7zM7 9c0-2.76 2.24-5 5-5s5 2.24 5 5c0 2.88-2.88 7.19-5 9.88C9.92 16.21 7 11.85 7 9z" /> | ||
<Circle cx="12" cy="9" r="2.5" /> | ||
</SVG> | ||
); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { __ } from '@wordpress/i18n'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import metadata from './block.json'; | ||
import icon from './icon'; | ||
import edit from './edit'; | ||
|
||
const { name } = metadata; | ||
export { metadata, name }; | ||
|
||
export const settings = { | ||
title: __( 'Site Title' ), | ||
icon, | ||
edit, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
<?php | ||
/** | ||
* Server-side rendering of the `core/site-title` block. | ||
* | ||
* @package WordPress | ||
*/ | ||
|
||
/** | ||
* Renders the `core/site-title` block on the server. | ||
* | ||
* @return string The render. | ||
*/ | ||
function render_block_core_site_title() { | ||
return sprintf( '<h1>%s</h1>', get_bloginfo( 'name' ) ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about linking this to the home? E.g.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe that should be an option/attribute? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's a very cool idea! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi, What happened with the site title linking to the home page? I can be done like this:
Cheers! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That could be a nice toggleable feature. Feel free to open an issue or PR for it 😄 |
||
} | ||
|
||
/** | ||
* Registers the `core/site-title` block on the server. | ||
*/ | ||
function register_block_core_site_title() { | ||
register_block_type( | ||
'core/site-title', | ||
array( | ||
'render_callback' => 'render_block_core_site_title', | ||
) | ||
); | ||
} | ||
add_action( 'init', 'register_block_core_site_title' ); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
<!-- wp:site-title /--> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
[ | ||
{ | ||
"clientId": "_clientId_0", | ||
"name": "core/site-title", | ||
"isValid": true, | ||
"attributes": {}, | ||
"innerBlocks": [], | ||
"originalContent": "" | ||
} | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
[ | ||
{ | ||
"blockName": "core/site-title", | ||
"attrs": {}, | ||
"innerBlocks": [], | ||
"innerHTML": "", | ||
"innerContent": [] | ||
}, | ||
{ | ||
"blockName": null, | ||
"attrs": {}, | ||
"innerBlocks": [], | ||
"innerHTML": "\n", | ||
"innerContent": [ | ||
"\n" | ||
] | ||
} | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
<!-- wp:site-title /--> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -100,7 +100,11 @@ describe( 'Block transforms', () => { | |
|
||
const transformStructure = {}; | ||
beforeAll( async () => { | ||
await enableExperimentalFeatures( [ '#gutenberg-widget-experiments', '#gutenberg-menu-block' ] ); | ||
await enableExperimentalFeatures( [ | ||
'#gutenberg-widget-experiments', | ||
'#gutenberg-menu-block', | ||
'#gutenberg-full-site-editing', | ||
] ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
await createNewPost(); | ||
|
||
for ( const fileBase of fileBasenames ) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since I've started to see this popping up in other PRs, what do you think if we add the Full Site Editing experiment plumbing in its own PR to make it easier to contribute towards this feature?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's already been merged. This is just adding missing default values and JSDoc comments.