DataViews: Removing mapping of user patterns to temporary object #63042
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
youknowriad
added
[Type] Code Quality
youknowriad
requested review from
Mamaduka,
oandregal,
talldan,
ellatrix,
jorgefilipecosta and
ntsekouras
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
I noticed that "uncategorized" patterns page is broken (it shows user pattern with categories) but I think it's broken on trunk too. |
|
Size Change: +662 B (+0.04%) Total Size: 1.76 MB
ℹ️ View Unchanged
|
youknowriad
force-pushed
the
update/register-reset-post-globally
branch
from
99a30cb
to
0c9a7c1
Compare
youknowriad
requested review from
nerrad,
ajitbohra,
ryanwelcher,
juanmaguitar,
fabiankaegy and
ndiego
as code owners
youknowriad
force-pushed
the
update/register-reset-post-globally
branch
from
0c9a7c1
to
bb81710
Compare
youknowriad
force-pushed
the
update/remove-user-pattern-mapping
branch
from
6b5377b
to
0539bc8
Compare
youknowriad
force-pushed
the
update/remove-user-pattern-mapping
branch
from
0539bc8
to
3a65582
Compare
--edit: I don't see this in trunk... |
ntsekouras
reviewed
Jul 4, 2024
| @@ -117,6 +117,7 @@ function PreviewWrapper( { item, onClick, ariaDescribedBy, children } ) { | |||
|
|
|||
| function Preview( { item, viewType } ) { | |||
| const descriptionId = useId(); | |||
| const description = item.description || item?.excerpt?.raw; | |||
There was a problem hiding this comment.
I know this was from before, but it surprized me to see raw and not rendered here..
There was a problem hiding this comment.
Same :) I think there's some consistency work to be done on the REST API.
ntsekouras
reviewed
Jul 4, 2024
| @@ -300,6 +301,8 @@ export default function DataviewsPatterns() { | |||
| header: __( 'Sync status' ), | |||
| id: 'sync-status', | |||
| render: ( { item } ) => { | |||
| const syncStatus = | |||
| item.wp_pattern_sync_status || PATTERN_SYNC_TYPES.full; | |||
There was a problem hiding this comment.
It seems we need to check if it's a user pattern, similarly with the search.
There was a problem hiding this comment.
If you see a few lines above, this field is only added for "user" patterns.
There was a problem hiding this comment.
Will check now, but the rendering of the label is wrong for patterns..
There was a problem hiding this comment.
I pushed a fix for this, but feel free to revert/update if you have a better solution..
ntsekouras
reviewed
Jul 5, 2024
youknowriad
commented
Jul 5, 2024
| @@ -302,23 +302,19 @@ export default function DataviewsPatterns() { | |||
| id: 'sync-status', | |||
| render: ( { item } ) => { | |||
| const syncStatus = | |||
| item.wp_pattern_sync_status || PATTERN_SYNC_TYPES.full; | |||
| 'wp_pattern_sync_status' in item | |||
There was a problem hiding this comment.
How can this condition be false if "type" is always user? (line 299)
There was a problem hiding this comment.
Probably it's when we merge the theme patterns with the user ones? You can see the issue without this change in categories like about, etc..
|
I fixed the uncategorized patterns issue. |
Thanks. I had a similar fix but hadn't pushed yet because I see one more problem. If we have created custom pattern categories, the sidebar doesn't render them. You can test that by creating a new pattern with a new category that didn't exist before. |
|
Good catch, it's fixed in the latest commit. |
|
This should be ready right? |
ntsekouras
approved these changes
Jul 5, 2024
|
This is the kind of code that could benefit from being typescripted :P |
t-hamano
reviewed
Jul 7, 2024
| content: item.patternPost.content.raw, | ||
| syncStatus: item.patternPost.wp_pattern_sync_status, | ||
| title: getItemTitle( item ), | ||
| content: item.content.raw, |
There was a problem hiding this comment.
In the DataViews, pattern content is indeed present in item.content.raw. But when exported from the sidebar, the content is present in item.content, so there seems to be no content field in the exported JSON file.
However, pattern export from the sidebar doesn't seem to have worked before. See this comment for more details.
There was a problem hiding this comment.
I have a small PR here:
carstingaxion
pushed a commit
to carstingaxion/gutenberg
that referenced
this pull request
Jul 18, 2024
…dPress#63042) Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: ntsekouras <ntsekouras@git.wordpress.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Similar to #62927 but for patterns
Related #59659
What?
When working on a the dataviews extensibility actions. I noticed that some of the post type actions receive objects that are not actual post types, they are temporary objects that were created for the purpose of rendering template parts in the patterns page. We were mapping user patterns to weird objects that resemble theme patterns.
For me this is not something we should be doing because:
How?
This PR just removes that mapping and tries to update all the places where that mapping was necessary. The code is a bit hard to follow, so there's a chance that I missed some places. Some help testing everything is needed.
Testing Instructions
1- Use the different filters, fields and actions in the patterns dataviews (and pattern editor).