Fix : Duplicative link control label and placeholder by Vrishabhsk · Pull Request #66329 · WordPress/gutenberg

Vrishabhsk · 2024-10-22T13:56:17Z

What?

Prevent duplicate text for label and placeholder props of LinkControlSearchInput component

Why?

Duplicative link control label and placeholder #66313

How?

Add label prop to LinkControlSearchInput component
The label prop get its value from searchInputLabel prop set for LinkControl component

github-actions · 2024-10-22T13:56:32Z

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: Vrishabhsk <vrishabhsk@git.wordpress.org>
Co-authored-by: getdave <get_dave@git.wordpress.org>
Co-authored-by: jeryj <jeryj@git.wordpress.org>
Co-authored-by: mirka <0mirka00@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
Co-authored-by: richtabor <richtabor@git.wordpress.org>

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

getdave

I appreciate the follow up here.

My main concern is that the new placeholder seems to just repeat the same text as the label. That seems superflous.

I would suggest we can remove the placeholder (by default).

MDN suggests the following definition of placeholder:

The placeholder text should provide a brief hint to the user as to the expected type of data that should be entered into the control.

Therefore using the word Search isn't terrible useful to the user as it's not an example of the type of input.

I thought about potential examples we could use. The two types of input are either:

A search string for an entity (e.g. a Post or Page) such as Sample Page.
A search for a URL such as www.example.com.

However the problem is that because it's in the @wordpress/block-editor package, this control can be used outside of WordPress. This rules out any WordPress-specific examples such as www.wordpress.org or Sample Page.

As a result Im now leaning towards defaulting placeholder to null and requiring that the specific implementation defines the placeholder. So when being used in a WP context the placeholder could be set.

Thanks again for your work here 👍

packages/block-editor/src/components/link-control/index.js

packages/block-editor/src/components/link-control/search-input.js

jeryj

@mirka Should the <InputBase/> implement logic to force the label to match the placeholder if hideLabelFromVision is true?

That seems like it would prevent a lot of bugs and force some built-in a11y.

jeryj · 2024-10-25T11:57:57Z

packages/block-editor/src/components/link-control/search-input.js

 		return (
 			<div className="block-editor-link-control__search-input-container">
 				<URLInput
 					disableSuggestions={ currentLink?.url === value }
-					label={ inputLabel }
+					label={ label ?? __( 'Search or type URL' ) }
 					hideLabelFromVision={ hideLabelFromVision }
 					className={ className }
 					value={ value }
 					onChange={ onInputChange }
-					placeholder={ inputLabel }
+					placeholder={ placeholder }


From the Why section of the PR that introduced the duplicative change, the rationale is that if there is no visible label then the placeholder text must match the label. It doesn't look like <InputBase /> handles this for us, so we could do it here:

Set a const for the inputLabel above this block: const inputLabel = label ?? __( 'Search or type URL' )

Apply the label: label={ inputLabel }

Check for hideLabelFromVision before applying the placeholder: placeholder={ hideLabelFromVision ? inputLabel : placeholder }

👍 We don't want to end up in a situation where we have the label and placeholder showing the same thing. That's the principle bug we have to address in this PR. Visual reference:

Yup - hopefully the sample code I added will:

Not have the placeholder and label match unless the consumer passes strings that match.

Force the placeholder to match the label only if hideLabelFromVision is true and we would want the placeholder and label to match.

mirka · 2024-10-25T14:25:47Z

Should the <InputBase/> implement logic to force the label to match the placeholder if hideLabelFromVision is true?

For a low-level component, this seems a bit too heavy-handed for me since hideLabelFromVision doesn't necessarily mean that the input has no visible label whatsoever. It could be that the consumer wants to hide the default-styled label, and replace it with a custom-styled one.

Though, I think we can start by including this accessibility guideline in the best practices section of the component docs that we're working on improving right now (for example in #66000).

t-hamano · 2025-03-17T16:10:24Z

It looks like this issue needs be fixed in the WP 6.8 release, so I'll add this PR to the project board.

t-hamano · 2025-03-19T07:09:28Z

The minimal PR to fix only the visually duplicative label and placeholder: #69620

t-hamano · 2025-03-19T11:11:15Z

Removing this PR from the 6.8 project board as an alternative PR has been prepared: #69620

Mamaduka · 2025-03-27T17:23:36Z

The original issue has been resolved. I'm going to close the PR.

Thanks for contributing, @Vrishabhsk!

t-hamano · 2025-03-28T08:23:49Z

Thanks to the folks who left feedback on this PR. I have submitted a new issue to explore what the ideal approach would be to address this issue.

#69652

Differentiate label and placeholder for LinkControl

1de0719

Vrishabhsk requested a review from getdave as a code owner October 22, 2024 13:56

Vrishabhsk mentioned this pull request Oct 22, 2024

Duplicative link control label and placeholder #66313

Closed

hbhalodia linked an issue Oct 23, 2024 that may be closed by this pull request

Duplicative link control label and placeholder #66313

Closed

Modify tests to handle new placeholder default

269b52a

getdave added [Type] Regression Related to a regression in the latest release [Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting) labels Oct 24, 2024

getdave reviewed Oct 24, 2024

View reviewed changes

packages/block-editor/src/components/link-control/index.js Outdated Show resolved Hide resolved

packages/block-editor/src/components/link-control/search-input.js Show resolved Hide resolved

getdave requested review from jeryj and richtabor October 24, 2024 08:36

getdave assigned Vrishabhsk Oct 24, 2024

Set placeholder's fb as null and update tests

b52ca86

jeryj reviewed Oct 25, 2024

View reviewed changes

Render placeholder based on hideLabelFromVision

b1f3ccd

getdave requested a review from jeryj November 4, 2024 12:42

Vrishabhsk requested a review from getdave December 27, 2024 06:46

t-hamano added this to WordPress 6.8 Editor Tasks Mar 17, 2025

github-project-automation bot moved this to 🔎 Needs Review in WordPress 6.8 Editor Tasks Mar 17, 2025

t-hamano mentioned this pull request Mar 19, 2025

LinkControl: Fix visually Duplicative label and placeholder #69620

Merged

t-hamano removed this from WordPress 6.8 Editor Tasks Mar 19, 2025

Mamaduka closed this Mar 27, 2025

Conversation

Vrishabhsk commented Oct 22, 2024

What?

Why?

How?

Uh oh!

github-actions bot commented Oct 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

getdave left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jeryj left a comment

Choose a reason for hiding this comment

Uh oh!

jeryj Oct 25, 2024

Choose a reason for hiding this comment

Uh oh!

getdave Oct 25, 2024

Choose a reason for hiding this comment

Uh oh!

jeryj Oct 25, 2024

Choose a reason for hiding this comment

Uh oh!

mirka commented Oct 25, 2024

Uh oh!

t-hamano commented Mar 17, 2025

Uh oh!

t-hamano commented Mar 19, 2025

Uh oh!

t-hamano commented Mar 19, 2025

Uh oh!

Mamaduka commented Mar 27, 2025

Uh oh!

t-hamano commented Mar 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

github-actions bot commented Oct 22, 2024 •

edited

Loading

getdave left a comment •

edited

Loading