Improve implementation of the 'Show button text labels' preference about gutenberg HOT 6 OPEN

I think the most difficult part would be the reliability of the automatic transform. Because Button unfortunately has a bunch of props, allowing consumers to set icons/text in different ways.

Yes I have the same concern as well. For this reason I'd tend to think the base component should be kept unchanged. The wrapper component option seems more reasnbale to me as it addresses a feature that is specific to the Gutenberg editor.

we'd have to replace all relevant Buttons in the Editor, some of which are already enclosed within a compound component.

Yes. I'm not sure it's a lot of files to touch, actually I have the impression it's a few files overall. Some of these compound component already use an as prop that would allow to just replace the Button component with the 'ButtonWrapper'. As per the impact, I'd think we'd have to try it to have a more clear picture.
Also important: the 'ButtonWrapper' could have test to catch any issue when the base Button component changes in any way.

I'll try and submit a Draft PR so to have some proof of contept to look at.

from gutenberg.

Here's the occurrences of the CSS selector .show-icon-labels I was able to find. At the time of writing there's several occurrences in 12 different files.

'Unlock' button in block toolbar 
https://github.com/WordPress/gutenberg/blob/2e439917b871eac2219b90c176ff4618c6e0852a/packages/block-editor/src/components/block-lock/style.scss#L62

Patterns explorer pagination:
https://github.com/WordPress/gutenberg/blob/2e439917b871eac2219b90c176ff4618c6e0852a/packages/block-editor/src/components/block-patterns-paging/style.scss#L25

Block switcher: small fix to prevent double label text 
https://github.com/WordPress/gutenberg/blob/2e439917b871eac2219b90c176ff4618c6e0852a/packages/block-editor/src/components/block-switcher/style.scss#L33

All buttons in the block toolbar, with padding override to 6px 
https://github.com/WordPress/gutenberg/blob/2e439917b871eac2219b90c176ff4618c6e0852a/packages/block-editor/src/components/block-toolbar/style.scss#L162

Block tools: 3 occurrences at line 198, 239, 247
various adjustments for block toolbar when the block has a parent
adjustments for block mover buttons
https://github.com/WordPress/gutenberg/blob/trunk/packages/block-editor/src/components/block-tools/style.scss

Block Inspector tabs 
https://github.com/WordPress/gutenberg/blob/2e439917b871eac2219b90c176ff4618c6e0852a/packages/block-editor/src/components/inspector-controls-tabs/style.scss#L1

LinkControl buttons 
https://github.com/WordPress/gutenberg/blob/2e439917b871eac2219b90c176ff4618c6e0852a/packages/block-editor/src/components/link-control/style.scss#L25

Inline format toolbar 
To my understanding, the inline format toolbar was removed in https://github.com/WordPress/gutenberg/pull/58945
Cc @youknowriad 
https://github.com/WordPress/gutenberg/blob/f25f59d8680b0c4d9b68d1f3aaa94e209f3b9b80/packages/block-editor/src/components/rich-text/style.scss#L33

Override for Post editor 'view posts' logo link in the toolbar when a site icon is set
https://github.com/WordPress/gutenberg/blob/f25f59d8680b0c4d9b68d1f3aaa94e209f3b9b80/packages/edit-post/src/components/header/style.scss#L4

Global styles panel header buttons
note: layout is currently broken 
https://github.com/WordPress/gutenberg/blob/72612dc94ce38ab2419669b3f0e844e515c975d1/packages/edit-site/src/components/global-styles-sidebar/style.scss#L83

Small adjustments fo the Inserter toggle button in the top bar 
Note: it seems some CSS properties are unnecessary 
Left amrgin adjustment for all Document tools buttons
https://github.com/WordPress/gutenberg/blob/72612dc94ce38ab2419669b3f0e844e515c975d1/packages/editor/src/components/document-tools/style.scss#L82

Editor header: 4 occurrences 
buttons in the editor top bar 
block mover when 'Top toolbar' is enabled 
pinned items - this should be checked, not sure the element 'interface-pinned-items' has also a 'show-icon-labels' CSS class
https://github.com/WordPress/gutenberg/blob/72612dc94ce38ab2419669b3f0e844e515c975d1/packages/editor/src/components/header/style.scss

from gutenberg.

Excited to see this! I’ve always wanted that setting to work more like MacOS toolbars.

from gutenberg.

The more I investigate this issue, the more I tend to think any CSS implementation is inherently fragile and not solid enough to make this feature reliable. Also, it's not well testable.

The current implementation uses a CSS pseudo element ::before or ::after depending whether one of the two is already used for other purposes. There are also edge cases, for example the inspector tabs, where the focus and active styles already use both ::before and ::after so that when 'Show button text labels' is enabled, the revealed text is slightly mis-positioned.

Other potential issues may arise from other potential CSS conflicts.

Fudnamentally, revealing the text by using CSS pseudo elements that target the button aria-label attribute might seem a smart idea at first but it's a fragile CSS hack. This feature should use a more solid implementation.

However, I do realize the base Button component should not support this as a built-in feature. I can think of two options and I'd greatly appreciate some feedback from the Components package maintainers and other Gutenberg specialists:

• Make the Button component support soem kind of generic transformation based on the environment the Button is used that is not specifically tied to 'Show button text labels'. As in: the environment may send in some way some props to the Button to instruct it to change in some way. This sounds a little against best practices though.
• Keep the Button component 'pure' and introduce a new 'preference-aware' button in the Editor component. This coomponent would basically be a wrapper of the base Button that is aware of the user preference and just passes the right props to not render the SVG icons and render text instead.

The second option seems cleaner ot me and wouldn't require any CSS hacks.
Cc @WordPress/gutenberg-core @WordPress/gutenberg-components

from gutenberg.

#46025 proposed not relying on global class names.

One approach I can think of is to change the behavior of the Button component ad hoc depending on whether this setting is enabled or not, without relying on the .show-icon-label class. That is, if this setting is enabled, it will render the actual text as a child instead of the icon:

function ParentComponent() {
	const showIconLabels = useSelect( ( select ) => {
		return select( preferencesStore ).get( 'core', 'showIconLabels' );
	}, [] );
	return (
		<>
			<ChildConponentOne showIconLabels={ showIconLabels } />
			<ChildConponentTwo showIconLabels={ showIconLabels } />
		</>
	);
}

function ChildConponentOne( { showIconLabels } ) {
	return (
		<Button
			label={ __( 'Push Me' ) }
			icon={ showIconLabels ? someIcon : undefined }
			showTooltip={ ! showIconLabels }
		>
			{ showIconLabels && __( 'Push Me' ) }
		</Button>
	);
}

function ChildConponenTwo( { showIconLabels } ) {
	return (
		<Button
			label={ __( 'Push Me' ) }
			icon={ showIconLabels ? someIcon : undefined }
			showTooltip={ ! showIconLabels }
		>
			{ showIconLabels && __( 'Push Me' ) }
		</Button>
	);
}

However, this will require changes to all UI that currently depend on the .show-icon-label class.

from gutenberg.

• Make the Button component support soem kind of generic transformation based on the environment the Button is used that is not specifically tied to 'Show button text labels'. As in: the environment may send in some way some props to the Button to instruct it to change in some way. This sounds a little against best practices though.

We can accomplish this using the Context System. For what it's worth, I'm not against this approach at all. On the other hand, the latter approach with the Editor-only wrapper seems unfeasible because we'd have to replace all relevant Buttons in the Editor, some of which are already enclosed within a compound component.

I think the most difficult part would be the reliability of the automatic transform. Because Button unfortunately has a bunch of props, allowing consumers to set icons/text in different ways.

from gutenberg.