-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Try/saving to theme files #40385
Try/saving to theme files #40385
Conversation
ca0cc07
to
33fdb62
Compare
Size Change: +36 B (0%) Total Size: 1.23 MB
ℹ️ View Unchanged
|
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.
Pull request authors can't request changes to their own pull request, but I want to request myself all these changes. Why would this be a problem ...
lib/compat/wordpress-6.1/class-gutenberg-rest-edit-site-export-controller.php
Show resolved
Hide resolved
lib/compat/wordpress-6.1/class-gutenberg-rest-edit-site-export-controller.php
Outdated
Show resolved
Hide resolved
packages/edit-site/src/components/header/theme-udpdater/index.js
Outdated
Show resolved
Hide resolved
packages/edit-site/src/components/header/theme-udpdater/index.js
Outdated
Show resolved
Hide resolved
packages/edit-site/src/components/header/theme-udpdater/index.js
Outdated
Show resolved
Hide resolved
33fdb62
to
dc33b3d
Compare
c6f1562
to
bd72aed
Compare
Simplified the API code as I don't have a good grasp of how child themes would need to be handled here, but this can be added once this is not a draft anymore. For now to not be a draft I need to:
|
…e code from zip export
bd72aed
to
66f25aa
Compare
Is it better to use this than the standard |
return $filename; | ||
} | ||
|
||
function gutenberg_export_theme_json() { |
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.
should we move this to a different file?
@@ -172,6 +173,7 @@ export default function Header( { | |||
|
|||
<div className="edit-site-header_end"> | |||
<div className="edit-site-header__actions"> | |||
<ThemeUpdater /> |
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.
I think this was better in the sidebar, like you had it before
lib/compat/wordpress-6.1/class-gutenberg-rest-edit-site-export-controller.php
Show resolved
Hide resolved
lib/compat/wordpress-6.1/class-gutenberg-rest-edit-site-export-controller.php
Outdated
Show resolved
Hide resolved
lib/compat/wordpress-6.1/class-gutenberg-rest-edit-site-export-controller.php
Show resolved
Hide resolved
return $template; | ||
} | ||
|
||
function clear_user_customizations() { |
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.
I wonder if we already have something that does this.
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.
Can we use this?
export const useGlobalStylesReset = () => { |
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.
Not right now as it returns a buggy canReset
which is always true. You can see that the reset menu item in the global styles panel is always active. But I'll see.
Should we add some tests? |
The workflow when editing is slightly cumbersome:
Would it be possible to give users the option to save changes directly to files without needing the intermediary step? |
@scruffian I've been thinking about either one of these paths: But I am not sure which way to go. Either way, I could enable the header button even if the theme templates and parts are none dirty and so the intermediary step could be gone. |
@mtias it would be good to get some design input here. Are you able to help or suggest someone who could? |
Yes, definitely. I usually like to have |
$global_styles_controller->update_item( $update_request ); | ||
|
||
// delete global styles transients | ||
delete_transient( 'global_styles' ); |
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.
I think this is obsolete.
// delete global styles transients | ||
delete_transient( 'global_styles' ); | ||
delete_transient( 'global_styles_' . get_stylesheet() ); | ||
delete_transient( 'gutenberg_global_styles' ); |
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.
I think this is obsolete.
const customTemplates = filter( templates, customSourceFilter ); | ||
const customTemplateParts = filter( templateParts, customSourceFilter ); | ||
|
||
const unModifiedTheme = |
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.
This condition also needs to check for changes to Global Styles.
} | ||
|
||
foreach ( $user_customisations['template_parts'] as $template_part ) { | ||
$fileToUpdate = get_template_directory() . '/parts/' . |
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.
In older themes this is called block-template-parts
. I don't think we necessarily need to account for that, but it is a limitation.
$user_customisations = $this->get_user_customisations(); | ||
|
||
foreach ( $user_customisations['templates'] as $template ) { | ||
$fileToUpdate = get_template_directory() . '/templates/' . |
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.
In older themes this is called block-templates
. I don't think we necessarily need to account for that, but it is a limitation.
// the result in an HTML comment". | ||
// This reverses that escaping for saving back to files. | ||
// @see `wp-includes/blocks.php` | ||
$template->content = str_replace( '\\u002d\\u002d', '--', $template->content ); |
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.
We took the decision not to do this when exporting the theme. If we're happy doing it here then I think we should also do the same for export.
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.
Indeed from #39576 looks like we should leave these encodings in place.
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.
Hmm the devex of working with code like this
{"bottom":"var(\u002d\u002dwp\u002d\u002dcustom\u002d\u002dspacing\u002d\u002dsmall, 1.25rem)"}}}}
is awful so we need to not do it.
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.
I agree, but if that's what we think we should make the same change in the export.
lib/compat/wordpress-6.1/class-gutenberg-rest-edit-site-export-controller.php
Show resolved
Hide resolved
…folder path, checks edit_themes cap, refactoring, batch check for file and folder permissions
@draganescu Can you tell me how I can use/test this with wp-env? The "Update Theme" button shows up in TT2, but I have no permissions to edit theme files. So I press the button and it gives me the error that the file permissions are not set correctly. And btw, for a manually added theme (skatepark) via config it actually does not give an error, it says "theme files exported" or something like that, and then it immediately resets to the original state so that does not seem to work. This is my modded config. {
"core": "WordPress/WordPress",
"plugins": [ "." ],
"themes": [ "./test/emptytheme", "/home/user/websites/s.com/web/app/themes/skatepark" ],
"config": { "ENABLE_THEME_SAVING": true },
"env": {
"tests": {
"mappings": {
"wp-content/plugins/gutenberg": ".",
"wp-content/mu-plugins": "./packages/e2e-tests/mu-plugins",
"wp-content/plugins/gutenberg-test-plugins": "./packages/e2e-tests/plugins",
"wp-content/themes/gutenberg-test-themes": "./packages/e2e-tests/themes"
}
}
}
} |
I figured out the reset messup, it actually DOES work for the manually provided theme folder it is just that the theme uses the |
@nextgenthemes for wpenv the only way is to use an outside linked theme, like you did with skatepark.
That actually looks like it is working :) To test have skatepark active, make a easily visible change, click update theme then open the template or part in a text editor and see if it did really change. |
I will close this in accordance with the details here #39194 (comment) |
What?
Closes #39194
Why?
To allow the site editor to behave like a traditional visual builder. A theme can be exported, but when developing from scratch a better devx is to update your theme's files with the changes made via the site editor.
How?
The following updates are needed:
themeDevMode
based on a newwp-config.php
constantTHEME_DEV_MODE
Testing Instructions
define( 'ENABLE_THEME_SAVING', true );
to yourwp-config.php
Screenshots or screencast
More menu
update-theme-more-menu.mp4
Editor header
update-theme-header.mp4
Work in progress, known problems
Based heavily on the create-block-theme plugin's implementation of this similar concept by @pbking