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.

Sign up for GitHub

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

Jump to bottom

Templates: Allow browsing by source, grouping all Custom templates #58946

Open
wants to merge 7 commits into
base: trunk
Choose a base branch
from
Open

Templates: Allow browsing by source, grouping all Custom templates #58946

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
de462a7
Data views: Use File icon for "All templates" view
mcsf Feb 9, 2024
8794a32
Data views: Group templates by source, collapsing custom ones
mcsf Feb 9, 2024
ebaef85
Add `original_source` field to support "Custom" filter
mcsf Feb 12, 2024
9e43201
Rename field to `source`. Simplify theme filtering
mcsf Feb 12, 2024
7f00290
Try: Remove `source` field definition, keep filtering logic
mcsf Feb 12, 2024
8dd4ebe
Rename "Custom" sidebar entry to "User"
mcsf Feb 13, 2024
14a264c
Try: Add "My templates" entry, revert source support
mcsf Feb 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 74 additions & 24 deletions packages/edit-site/src/components/page-templates-template-parts/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ const DEFAULT_VIEW = {
},
// All fields are visible by default, so it's
// better to keep track of the hidden ones.
hiddenFields: [ 'preview' ],
hiddenFields: [ 'preview', 'source' ],
layout: defaultConfigPerViewType[ LAYOUT_TABLE ],
filters: [],
};
Expand Down Expand Up @@ -188,6 +188,33 @@ function Preview( { item, viewType } ) {
);
}

function computeFilters( activeView ) {
// activeView takes values such as:
// - 'all'
// - 'user'
// - 'theme'
// - 'plugin:My Plugin'
switch ( activeView ) {
case 'all':
return [];

case 'user':
case 'theme':
return [ { field: 'source', operator: 'in', value: activeView } ];

default: {
const idx = activeView.indexOf( ':' );
const source = activeView.substring( 0, idx );
const author = activeView.substring( idx + 1 );
const filters = [
{ field: 'source', operator: 'in', value: source },
{ field: 'author', operator: 'in', value: author },
];
return filters;
}
}
}

export default function PageTemplatesTemplateParts( { postType } ) {
const { params } = useLocation();
const { activeView = 'all', layout } = params;
Expand All @@ -199,32 +226,14 @@ export default function PageTemplatesTemplateParts( { postType } ) {
...DEFAULT_VIEW,
type: usedType,
layout: defaultConfigPerViewType[ usedType ],
filters:
activeView !== 'all'
? [
{
field: 'author',
operator: 'in',
value: activeView,
},
]
: [],
filters: computeFilters( activeView ),
};
}, [ layout, activeView ] );
const [ view, setView ] = useState( defaultView );
useEffect( () => {
setView( ( currentView ) => ( {
...currentView,
filters:
activeView !== 'all'
? [
{
field: 'author',
operator: 'in',
value: activeView,
},
]
: [],
filters: computeFilters( activeView ),
} ) );
}, [ activeView ] );

Expand Down Expand Up @@ -262,6 +271,12 @@ export default function PageTemplatesTemplateParts( { postType } ) {
} ) );
}, [ records ] );

const sources = useMemo( () => ( {
theme: __( 'Theme' ),
plugin: __( 'Plugin' ),
user: __( 'User' ),
Copy link
Contributor

Choose a reason for hiding this comment

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

In the sidebar of the site editor, it says "custom" and here it says "user". I believe we should use the same label no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question, that's one thing I'd like to get feedback on:

  • In the sidebar, "Custom" feels more intuitive to me. In WP jargon we typically see "custom templates", and rarely "user templates". We could switch to "User", but that feels confusing.
  • However, in the filter, Source is Custom felt like a weird rendering, so I kept it true to the internal naming: Source is User.

What should we do? Both directions seem weird to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think "User" templates in the sidebar is fine by me too. But would love feedback from @jameskoster

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the ping. Given this filter shows any user-generated templates, I agree it's fine for the label in the sidebar to be 'User'.

However, it would be good to have a 'Custom' view as well, which should show any $custom templates according to the template hierarchy. This would match the 'Custom' section in the current template list. I acknowledge there's no way to filter in this manner yet, so I guess it'll need to happen separately. Perhaps we should add a 'type' taxonomy?

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean if the goal is to have separation according to "custom", "non custom" templates, that's possible to, it's just a different field. But the question is also what to do for "plugin templates" because these also can be custom or not.

So I guess before moving on here, let's get some clarity about what we want for 6.5 from a design perspective.

Copy link
Contributor

Choose a reason for hiding this comment

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

If a plugin (or theme) template is custom then it should appear in the "Custom" view, just as those templates appear currently under the "Custom" heading in the Templates sidebar:

Screenshot 2024-02-12 at 21 07 26

For 6.5, at the least, I would suggest replicating the current list, IE a view for custom templates, and a view for each plugin that adds templates. Source = user is a nice-to-have, but since it doesn't exist already not essential.

} ) );

const fields = useMemo( () => {
const _fields = [
{
Expand Down Expand Up @@ -329,8 +344,27 @@ export default function PageTemplatesTemplateParts( { postType } ) {
elements: authors,
width: '1%',
} );
_fields.push( {
header: __( 'Source' ),
id: 'source',
getValue: ( { item } ) => item.original_source,
render: ( { item } ) => {
return (
<span className="page-templates-source">
{ sources[ item.original_source ] }
</span>
);
},
type: ENUMERATION_TYPE,
elements: Object.entries( sources ).map( ( [ value, label ] ) => ( {
value,
label,
} ) ),
enableHiding: false,
enableSorting: false,
} );
return _fields;
}, [ postType, authors, view.type ] );
}, [ postType, authors, sources, view.type ] );

const { data, paginationInfo } = useMemo( () => {
if ( ! records ) {
Expand Down Expand Up @@ -362,19 +396,35 @@ export default function PageTemplatesTemplateParts( { postType } ) {
if (
filter.field === 'author' &&
filter.operator === OPERATOR_IN &&
!! filter.value
filter.value
) {
filteredData = filteredData.filter( ( item ) => {
return item.author_text === filter.value;
} );
} else if (
filter.field === 'author' &&
filter.operator === OPERATOR_NOT_IN &&
!! filter.value
filter.value
) {
filteredData = filteredData.filter( ( item ) => {
return item.author_text !== filter.value;
} );
} else if (
filter.field === 'source' &&
filter.operator === OPERATOR_IN &&
filter.value
) {
filteredData = filteredData.filter( ( item ) => {
return item.original_source === filter.value;
} );
} else if (
filter.field === 'source' &&
filter.operator === OPERATOR_NOT_IN &&
filter.value
) {
filteredData = filteredData.filter( ( item ) => {
return item.original_source !== filter.value;
} );
}
} );
}
Expand Down
106 changes: 66 additions & 40 deletions packages/edit-site/src/components/sidebar-navigation-screen-templates-browse/content.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,29 +4,18 @@
import { useEntityRecords } from '@wordpress/core-data';
import { useMemo } from '@wordpress/element';
import { __experimentalItemGroup as ItemGroup } from '@wordpress/components';
import { __ } from '@wordpress/i18n';

/**
* Internal dependencies
*/
import DataViewItem from '../sidebar-dataviews/dataview-item';
import { useAddedBy } from '../list/added-by';
import { layout } from '@wordpress/icons';

const EMPTY_ARRAY = [];

function TemplateDataviewItem( { template, isActive } ) {
const { text, icon } = useAddedBy( template.type, template.id );
return (
<DataViewItem
key={ text }
slug={ text }
title={ text }
icon={ icon }
isActive={ isActive }
isCustom="false"
/>
);
}
import {
file as fileIcon,
layout as themeIcon,
commentAuthorAvatar as customIcon,
plugins as pluginIcon,
} from '@wordpress/icons';

export default function DataviewsTemplatesSidebarContent( {
activeView,
Expand All @@ -36,38 +25,75 @@ export default function DataviewsTemplatesSidebarContent( {
const { records } = useEntityRecords( 'postType', postType, {
per_page: -1,
} );
const firstItemPerAuthorText = useMemo( () => {
const firstItemPerAuthor = records?.reduce( ( acc, template ) => {
const author = template.author_text;
if ( author && ! acc[ author ] ) {
acc[ author ] = template;

const sources = useMemo( () => {
if ( records ) {
const _sources = {
theme: undefined,
user: undefined,
plugin: {},
};

for ( const template of records ) {
const src = template.original_source;
const obj = src === 'plugin' ? _sources.plugin : _sources;
if ( ! obj[ src ] ) {
obj[ src ] = {
count: 0,
value: template.author_text,
};
}
obj[ src ].count++;
}
return acc;
}, {} );
return (
( firstItemPerAuthor && Object.values( firstItemPerAuthor ) ) ??
EMPTY_ARRAY
);

return _sources;
}
}, [ records ] );

return (
<ItemGroup>
<DataViewItem
slug={ 'all' }
slug="all"
title={ config[ postType ].title }
icon={ layout }
icon={ fileIcon }
isActive={ activeView === 'all' }
isCustom="false"
suffix={ <span>{ records?.length }</span> }
/>
{ firstItemPerAuthorText.map( ( template ) => {
return (
<TemplateDataviewItem
key={ template.author_text }
template={ template }
isActive={ activeView === template.author_text }
/>
);
} ) }
{ sources?.theme && (
<DataViewItem
slug="theme"
title={ __( 'Theme' ) }
icon={ themeIcon }
isActive={ activeView === 'theme' }
isCustom="false"
suffix={ <span>{ sources.theme.count }</span> }
/>
) }
{ sources?.user && (
<DataViewItem
slug="user"
title={ __( 'Custom' ) }
icon={ customIcon }
isActive={ activeView === 'user' }
isCustom="false"
suffix={ <span>{ sources.user.count }</span> }
/>
) }
{ sources?.plugin &&
Object.entries( sources.plugin ).map(
( [ key, { count, value } ] ) => (
<DataViewItem
key={ key }
slug={ `plugin:${ value }` }
title={ value }
icon={ pluginIcon }
isActive={ activeView === `plugin:${ value }` }
isCustom="false"
suffix={ <span>{ count }</span> }
/>
)
) }
</ItemGroup>
);
}
Loading