Icons Registry: Don't expose "internal" icons

[mcsf]

What?

Follow-up of #72215

Follow @jasmussen's recommendation to try to distinguish icons meant for internal use only and those we'd like to expose to consumers such as the Icon block (#71227), i.e. those icons that users are more likely to want to use in the front end:

How?

• In the icons manifest, introduce an optional array property public
• In the icons registry, ignore any manifest entry that doesn't have 'public' => true

Why not simply remove them from the manifest?

We can, but by keeping them (and adding this property) we can keep our validate-collection script, which helps us ensure that the manifest and the SVG files never go out of sync. This seems like a better tradeoff.

Testing Instructions

(outdated) To properly visualise the icons grid, apply the following patch — which is a modified version of @t-hamano's snippet from #75273 — then, in a fresh post editor, insert block "Icon List Test".

(outdated snippet)

diff --git a/lib/experimental/class-wp-icons-registry.php b/lib/experimental/class-wp-icons-registry.php
index aefd83f718c..7ce27c23eca 100644
--- a/lib/experimental/class-wp-icons-registry.php
+++ b/lib/experimental/class-wp-icons-registry.php
@@ -63,8 +63,10 @@ if ( ! class_exists( 'WP_Icons_Registry' ) ) {
 					continue;
 				}
 
+				$tail = $status !== '' ? '--ignored' : '';
+
 				$this->register(
-					'core/' . $icon_name,
+					'core/' . $icon_name . $tail,
 					array(
 						'label'    => $icon_data['label'],
 						'filePath' => $icons_directory . $icon_data['filePath'],
diff --git a/packages/block-library/src/index.js b/packages/block-library/src/index.js
index f891ec38c0a..422b141db18 100644
--- a/packages/block-library/src/index.js
+++ b/packages/block-library/src/index.js
@@ -446,3 +446,55 @@ export const __experimentalRegisterExperimentalCoreBlocks =
 		: undefined;
 
 export { privateApis } from './private-apis';
+
+wp.blocks.registerBlockType( 'test/icon-list', {
+	apiVersion: 3,
+	title: 'Icon List Test',
+	edit: class Edit extends wp.element.Component {
+		constructor( props ) {
+			super( props );
+			this.state = { icons: [] };
+		}
+		componentDidMount() {
+			wp.apiFetch( { path: '/wp/v2/icons' } ).then( ( data ) => {
+				this.setState( {
+					icons: Array.isArray( data ) ? data : [],
+				} );
+			} );
+		}
+		render() {
+			return wp.element.createElement(
+				'div',
+				{},
+				wp.element.createElement(
+					'div',
+					{
+						style: {
+							display: 'grid',
+							gridTemplateColumns:
+								'repeat(21, minmax(32px, 1fr))',
+							gap: '4px',
+							padding: 0,
+						},
+					},
+					this.state.icons.map( ( icon ) =>
+						wp.element.createElement( 'div', {
+							key: icon.name,
+							style: {
+								width: 32,
+								height: 32,
+								opacity: icon.name.endsWith( '--ignored' )
+									? 0.3
+									: 1,
+							},
+							title: icon.name,
+							dangerouslySetInnerHTML: {
+								__html: icon.content || '',
+							},
+						} )
+					)
+				)
+			);
+		}
+	},
+} );

To view the full set, including disabled icons, comment out the statement continue; in WP_Icons_Registry::__construct where it ignores hidden/disabled icons.

Screenshots or screencast

Comparison of expected and actual grid:

[github-actions]

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: mcsf <mcsf@git.wordpress.org>
Co-authored-by: jasmussen <joen@git.wordpress.org>
Co-authored-by: ellatrix <ellatrix@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[jasmussen]

Thanks for the ping, thanks for the work!

On whether to land this or not, I'll entirely defer to project leads, including yourself. I'm a big fan of the Icons block, and SVG support in WordPress, I think the best way to move this work forward is to land it as soon as possible, so we can iterate and improve. And it certainly needs a default icon set for that to happen, and the WordPress one is an obvious one.

The main reason I'm cautious, and wanting to start with as little as possible—perhaps even fewer than I shared in that image you included here—does not have to do with my skepticism of the feature, it's purely from a "start with less" philosophy based in the fact that adding icons is trivial, but removing them once added is virtually impossible. In that, a related question worth answering for this PR: what if we mean to deprecate an icon? Coming to mind, we have two "copy" icons, a big one and a small one, and they're minimally different so IMO we should deprecate one of them. Would that be harder/impossible if those icons were included in this package? If yes, we probably want to always defer to not including, start with as little as possible that is still meaningful, and then add on a per-icon basis once this is out there. The icon I'm referring to here is this one:

We should probably omit that as well, given what I outlined above.

We can presumably iterate/improve icons, if they maintain the same meaning going forward? Or does that also lock in? This is important for narrowing down to even fewer icons if the flexibility is limited.

[mcsf]

The main reason I'm cautious, and wanting to start with as little as possible—perhaps even fewer than I shared in that image you included here—does not have to do with my skepticism of the feature, it's purely from a "start with less" philosophy based in the fact that adding icons is trivial, but removing them once added is virtually impossible.

This is exactly how I feel too.

In that, a related question worth answering for this PR: what if we mean to deprecate an icon? Coming to mind, we have two "copy" icons, a big one and a small one, and they're minimally different so IMO we should deprecate one of them. Would that be harder/impossible if those icons were included in this package? If yes, we probably want to always defer to not including, start with as little as possible that is still meaningful, and then add on a per-icon basis once this is out there.

Right, deprecating will be tricky, and it's always easier to add than remove. The problem isn't adding icons to the package (since the package is a bundled one, it doesn't get exposed as window.wp.icons, and consumers of the package can use NPM versioning to avoid breaking changes). The problem is adding them to the icons registry, after which they are available to API consumers.

I have some preliminary ideas on handling icons deprecations. We don't need a plan right away, but I'm just sharing for completeness:

• Once icon core/foo is marked as as deprecated:
  • When making a request for all icons, i.e. to /wp/v2/icons, the icon will no longer be listed there
  • But nothing changes when making an API request to /wp/v2/icons/core/foo
    • We may call _doing_it_wrong to log a deprecation warning, but probably not: this isn't an API deprecation, it's closer to user content. So we probably just silently serve the deprecated icon forever.

So yes, I'd default to not including icons from the start.

We can presumably iterate/improve icons, if they maintain the same meaning going forward? Or does that also lock in? This is important for narrowing down to even fewer icons if the flexibility is limited.

In my opinion, we can and should iterate on existing icons as long as their meaning is preserved.

Of the two Copy icons, currently only copy-small is exposed, not copy. Is that still how you'd like it?

[ellatrix]

In my opinion, we can and should iterate on existing icons as long as their meaning is preserved.

Imo, we'd have to version them when making edits. We shouldn't cause updates to the front-end of people's sites?

[github-actions]

Flaky tests detected in 942da47.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/22069707677
📝 Reported issues:

• [Flaky Test] Should render and migrate legacy openSubmenusOnClick blocks #75559 in /test/e2e/specs/editor/blocks/navigation-frontend-interactivity.spec.js

[ellatrix]

Makes sense to me to be cautious and to exclude icons rather than exposing everything.

[mcsf]

In my opinion, we can and should iterate on existing icons as long as their meaning is preserved.

Imo, we'd have to version them when making edits. We shouldn't cause updates to the front-end of people's sites?

Isn't this what happens with all of our dynamic blocks? And also with our static-but-server-enhanced blocks. They are subject to change, as long as no user content is ever lost and as long as the changes are reasonable and don't change the meaning of anything.

Likewise, themes evolve, and in that process they can change the front end.

[ellatrix]

Right, I think you convinced me. :)

[jasmussen]

Of the two Copy icons, currently only copy-small is exposed, not copy. Is that still how you'd like it?

I'd like to actually remove both, for now. Here's an updated list that's even more conservative, based on the conversation here.

All that sounds very cautious, I know, but I'm sharing it with excitement. There's fresh energy on icons, and I personally think this means all those icon changes we've been wanting to do for a while gets new priority.