Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upGitHub is where the world builds software
Millions of developers and companies build, ship, and maintain their software on GitHub — the largest and most advanced development platform in the world.
Site Editor - Nav Panel - Only show auto-draft template parts corresponding to current theme. #26948
Conversation
Addison-Stavlo
requested review from
david-szabo97,
vindl,
noahtallen,
mattwiebe,
Copons and
jeyip
| per_page: -1, | ||
| } | ||
| ); | ||
| const currentTheme = select( 'core' ).getCurrentTheme()?.stylesheet; | ||
| const themeTemplateParts = select( 'core' ).getEntityRecords( |
Addison-Stavlo
Nov 12, 2020
Author
Contributor
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?
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?
Addison-Stavlo
Nov 12, 2020
Author
Contributor
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.
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.
noahtallen
Nov 12, 2020
Member
I don't have a strong opinion here, but the current two queries approach makes sense to me assuming that issue gets resolved
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
Nov 13, 2020
Author
Contributor
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.
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.
|
Size Change: +56 B (0%) Total Size: 1.19 MB
|
| 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 |
|
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! |
...c/components/navigation-sidebar/navigation-panel/menus/template-parts.js
Outdated
Show resolved
Hide resolved
| per_page: -1, | ||
| } | ||
| ); | ||
| const currentTheme = select( 'core' ).getCurrentTheme()?.stylesheet; | ||
| const themeTemplateParts = select( 'core' ).getEntityRecords( |
noahtallen
Nov 12, 2020
Member
I don't have a strong opinion here, but the current two queries approach makes sense to me assuming that issue gets resolved
I don't have a strong opinion here, but the current two queries approach makes sense to me assuming that issue gets resolved
Agreed. |
|
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 |
|
yep, works well for me! |
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?
Screenshots
Types of changes
Checklist: