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

FSE - Follow-up to #25995: Use WP_Theme::get_stylesheet instead of the private property. #26275

Closed
wants to merge 111 commits into from
Closed

FSE - Follow-up to #25995: Use WP_Theme::get_stylesheet instead of the private property. #26275

wants to merge 111 commits into from

Conversation

fklein-lu
Copy link
Contributor

Description

As noted in this comment on PR #25995, the WP_Theme::get_stylesheet() method should be used instead of of accessing the private property.

Code like wp_get_theme()->stylesheet only works because it is implemented as part of the __get() method. PHP IDE's and code analysis tools will flag this as a pattern to avoid.

How has this been tested?

Manually tested by verifying that template parts are still correctly being displayed on the frontend.

Types of changes

PHP code style enhancement.

ntsekouras and others added 11 commits October 19, 2020 17:47
* Fix tagline text alignment

* pass className in useBlockProps
* Draft batch-processing module

* committing/rudimentary API_FETCH integration

* Make batch processing functional!

* Roll back integration changes

* Cleanup package.json

* Update package-lock.json

* Update terminology

* yield from commitTransaction

* Correct yield from

* Move the comment where it belongs

* First pass at enqueueItemAndWaitForResults.

* Draft batch-processing module

* committing/rudimentary API_FETCH integration

* Make batch processing functional!

* Roll back integration changes

* Cleanup package.json

* Update package-lock.json

* Update terminology

* yield from commitTransaction

* Correct yield from

* Move the comment where it belongs

* update package-lock.json

* First pass at using the new sidebars and widget endpoints. (#26086)

* Draft batch-processing module

* committing/rudimentary API_FETCH integration

* Make batch processing functional!

* Roll back integration changes

* Cleanup package.json

* Update package-lock.json

* Update terminology

* yield from commitTransaction

* Correct yield from

* Move the comment where it belongs

* First pass at enqueueItemAndWaitForResults.

* First pass at using the new sidebars and widget endpoints.

* Add test for saving multiple widgets in a row

* Generate better error message when creating fails

* Add basic batch integration.

Works off of #26205.

* Allow mixing HTTP methods in a batch.

* Fix disappearing data, add basic error handling.

We need to get the full list of widget ids from the client, we can't use the
returned widget ids from the batch because not all widgets will be dirty.

Also implements basic error handling for when the batch fails. A snackbar is
added with the names of the widgets that failed to save. We also now propagate
a dummy error to the apiFetch callers for the requests that didn't have a
validation error, but need something to short circuit their handling.

* yield from the widget area saving.

* Move batch-processing into edit-widgets package

Co-authored-by: Adam Zieliński <[email protected]>

* remove separate batch-processing package

* Update batch processing tests

* Resolve batch errors in a safe, well-ordered way

* Fix unit tests

* Wire progress indicator to specific widgets

* Fix authorship after merge/rebase (1/2)

* Fix authorship after merge/rebase (2/2)

* Don't mark the method property as required.

We don't evaluate nested defaults yet, so this caused errors.

* Allow updating widgets without including the id_base or number.

* Don't start GET /wp/v2/widgets?per_page=-1 if there are batch requests already in progress

* Process newWidgetClientIds in correct order

* Fix package.json

* remove batch-processing package

Co-authored-by: Timothy Jacobs <[email protected]>
The `LegacyWidgetEditHandler` component now uses the widget-types endpoint.

Co-authored-by: Timothy Jacobs <[email protected]>
Copy link
Contributor

@Addison-Stavlo Addison-Stavlo left a comment

Choose a reason for hiding this comment

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

Thanks for picking this up, it looks good and works well in testing.

I noticed another place that we forgot to update from using textDomain that may be worth adding in the changes here:

$template_part_string = '<!-- wp:template-part {"slug":"' . $slug . '","theme":"' . wp_get_theme()->get( 'TextDomain' ) . '"} -->' . $content . '<!-- /wp:template-part -->';

There it is still using textDomain as the theme name when creating a draft of the template part. It has been working fine in testing since for these block based themes textDomain is currently set up to be the same as the stylesheet name, but we should probably make it consistent. 😁

TimothyBJacobs and others added 11 commits October 19, 2020 22:25
This let's people opt-in to a specific version of batching, and allows us to
introduce in the future further flags to customize behavior.
* Fix current_parsed_blocks with inner blocks

* Fix linting issues

Co-authored-by: Marcus Kazmierczak <[email protected]>
* hide overlay opacity control if there is no overlay color or gradient

* remove default black background color of has-background-dim class

* combine selectors for overlay opacity

* make overlay opacity range step congruent with available styles
Follow up to #24599.

Using the child combinator (instead of the descendant combinator) narrows the selector without increasing its specificity. This should help reduce the likelihood of a style issue occurring when a parent element has an `is-style-outline` CSS class.

https://developer.mozilla.org/en-US/docs/Web/CSS/Child_combinator
* Stabilize batching endpoint as v1.

* Update polyfill batching to allow for the widgets endpoint to work.
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
 - @wordpress/[email protected]
@fklein-lu
Copy link
Contributor Author

Messed up that rebase, clean PR in #26539.

@fklein-lu fklein-lu closed this Oct 28, 2020
@fklein-lu fklein-lu deleted the fix/25995-pr-feedback branch October 28, 2020 12:33
@Addison-Stavlo
Copy link
Contributor

@Addison-Stavlo or @vindl – it's fine to merge approved PRs for contributors that aren't members of the project and can't really do it themselves 😄

Oops! Yes, i completely overlooked that. Will do. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.