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

Site Editor - Nav Panel - Only show auto-draft template parts corresponding to current theme. #26948

Merged

Conversation

Addison-Stavlo
Copy link
Contributor

Description

As noted in #26402 if you switch themes a few times, auto-drafts of template parts build up and they are all shown in the Nav panel. This PR updates the nav panel to only show auto-drafts that correspond to the currently active theme.

How has this been tested?

  • Activate a block based theme, load the site editor.
  • Switch to another block based theme, load the site editor again.
  • verify the first themes template part auto-drafts are not present in the nav panel.

Screenshots

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

per_page: -1,
}
);
const currentTheme = select( 'core' ).getCurrentTheme()?.stylesheet;
const themeTemplateParts = select( 'core' ).getEntityRecords(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

alternatively, instead of doing 2 queries and combining the results we could do one query to retrieve all publish/auto-drafts and then filter the results to exclude auto drafts of other themes. I wonder which may be a better option?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems having the 2 queries here both firing as the site editor starts loading sometimes messes up auto-draft creation and we get a duplicate of each template part supplied by the theme. I would expect changes from #26383 to solve this since auto-drafts will have already been created by this point, but that still leaves the question above - combining 2 queries vs. filtering the results of 1 query.

Copy link
Member

Choose a reason for hiding this comment

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

I don't have a strong opinion here, but the current two queries approach makes sense to me assuming that issue gets resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I thought that PR was good to merge but it looks like you pointed out some other bugs with it.

I updated this PR to use the other approach to filter the results of one query instead, and it seems to be working well.

@github-actions
Copy link

github-actions bot commented Nov 12, 2020

Size Change: +56 B (0%)

Total Size: 1.19 MB

Filename Size Change
build/edit-site/index.js 23.4 kB +56 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.77 kB 0 B
build/api-fetch/index.js 3.42 kB 0 B
build/autop/index.js 2.84 kB 0 B
build/blob/index.js 664 B 0 B
build/block-directory/index.js 8.71 kB 0 B
build/block-directory/style-rtl.css 943 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-editor/index.js 133 kB 0 B
build/block-editor/style-rtl.css 11.1 kB 0 B
build/block-editor/style.css 11.1 kB 0 B
build/block-library/editor-rtl.css 8.91 kB 0 B
build/block-library/editor.css 8.91 kB 0 B
build/block-library/index.js 147 kB 0 B
build/block-library/style-rtl.css 8.1 kB 0 B
build/block-library/style.css 8.1 kB 0 B
build/block-library/theme-rtl.css 792 B 0 B
build/block-library/theme.css 793 B 0 B
build/block-serialization-default-parser/index.js 1.87 kB 0 B
build/block-serialization-spec-parser/index.js 3.06 kB 0 B
build/blocks/index.js 48 kB 0 B
build/components/index.js 171 kB 0 B
build/components/style-rtl.css 15.3 kB 0 B
build/components/style.css 15.3 kB 0 B
build/compose/index.js 9.9 kB 0 B
build/core-data/index.js 14.8 kB 0 B
build/data-controls/index.js 821 B 0 B
build/data/index.js 8.74 kB 0 B
build/date/index.js 11.2 kB 0 B
build/deprecated/index.js 768 B 0 B
build/dom-ready/index.js 571 B 0 B
build/dom/index.js 4.92 kB 0 B
build/edit-navigation/index.js 11.1 kB 0 B
build/edit-navigation/style-rtl.css 881 B 0 B
build/edit-navigation/style.css 885 B 0 B
build/edit-post/index.js 306 kB 0 B
build/edit-post/style-rtl.css 6.43 kB 0 B
build/edit-post/style.css 6.42 kB 0 B
build/edit-site/style-rtl.css 3.95 kB 0 B
build/edit-site/style.css 3.95 kB 0 B
build/edit-widgets/index.js 26.3 kB 0 B
build/edit-widgets/style-rtl.css 3.16 kB 0 B
build/edit-widgets/style.css 3.16 kB 0 B
build/editor/editor-styles-rtl.css 476 B 0 B
build/editor/editor-styles.css 478 B 0 B
build/editor/index.js 42.5 kB 0 B
build/editor/style-rtl.css 3.85 kB 0 B
build/editor/style.css 3.85 kB 0 B
build/element/index.js 4.62 kB 0 B
build/escape-html/index.js 735 B 0 B
build/format-library/index.js 6.86 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 2.16 kB 0 B
build/html-entities/index.js 623 B 0 B
build/i18n/index.js 3.57 kB 0 B
build/is-shallow-equal/index.js 713 B 0 B
build/keyboard-shortcuts/index.js 2.52 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.1 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.32 kB 0 B
build/notices/index.js 1.77 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.43 kB 0 B
build/priority-queue/index.js 790 B 0 B
build/redux-routine/index.js 2.83 kB 0 B
build/reusable-blocks/index.js 3.05 kB 0 B
build/rich-text/index.js 13.3 kB 0 B
build/server-side-render/index.js 2.77 kB 0 B
build/shortcode/index.js 1.69 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.84 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.22 kB 0 B

compressed-size-action

Copy link
Member

@noahtallen noahtallen left a comment

Choose a reason for hiding this comment

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

This works for me, but it'd be good too make sure that the auto-draft issue is fixed by the other PR before merging!

per_page: -1,
}
);
const currentTheme = select( 'core' ).getCurrentTheme()?.stylesheet;
const themeTemplateParts = select( 'core' ).getEntityRecords(
Copy link
Member

Choose a reason for hiding this comment

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

I don't have a strong opinion here, but the current two queries approach makes sense to me assuming that issue gets resolved

@Addison-Stavlo
Copy link
Contributor Author

but it'd be good too make sure that the auto-draft issue is fixed by the other PR before merging!

Agreed.

@Addison-Stavlo
Copy link
Contributor Author

Updated to use one query and filter the results, since the PR that would fix the bugs that happen when combining the two queries looks like it might still need some more time (I previously thought that one was ready to merge).

Let me know if this still looks good to you @noahtallen

Copy link
Member

@noahtallen noahtallen left a comment

Choose a reason for hiding this comment

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

yep, works well for me!

@Addison-Stavlo Addison-Stavlo merged commit 403fa60 into master Nov 16, 2020
@Addison-Stavlo Addison-Stavlo deleted the fix/old-theme-auto-draft-template-parts-in-nav-menu branch November 16, 2020 15:40
@github-actions github-actions bot added this to the Gutenberg 9.4 milestone Nov 16, 2020
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.

3 participants