Skip to content

Fix URL input fields to reserve space for spinner icon and prevent text overlap #70134

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.

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

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

Conversation

sarthaknagoshe2002
Copy link
Contributor

What?

This PR addresses a UI issue where the spinner icon in URL input fields overlaps with and hides long user-entered text.
Closes #58679

Why?

URL input field do not reserve space for the spinner icon. As a result, during loading states, long URLs are partially hidden, which disrupts user experience and readability.

How?

A simple padding was needed towards the left of the spinner

Testing Instructions

  1. Add a block (like Paragraph or Image) and start inserting a link.
  2. In the URL input field, type a long URL.
  3. Wait for the loading spinner to appear.
  4. Verify that the spinner no longer overlaps the URL text.
  5. Confirm layout stability and readability across both components.

Screenshots or screencast

Before After
image Screenshot 2025-05-15 at 6 30 56 PM

Copy link

github-actions bot commented May 15, 2025

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: sarthaknagoshe2002 <[email protected]>
Co-authored-by: t-hamano <[email protected]>
Co-authored-by: jasmussen <[email protected]>
Co-authored-by: afercia <[email protected]>

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

@t-hamano t-hamano added [Type] Bug An existing feature does not function as intended [Package] Block editor /packages/block-editor labels May 16, 2025
Copy link
Contributor

@t-hamano t-hamano left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

When defining styles, it's important to note the following:

  • Don't use selectors with the components- prefix as much as possbile: the internal implementation of the component may change.
  • Don't use hard-coded values ​​for spacing. Use 8px-based $grid-unit-xx.
  • Avoid using !important as much as possible.

I hope you'll explore approaches that meet these requirements. Additionally, right padding should not be necessary when the spinner is not visible.

@sarthaknagoshe2002
Copy link
Contributor Author

sarthaknagoshe2002 commented May 22, 2025

Additionally, right padding should not be necessary when the spinner is not visible.

@t-hamano I experimented with this, and I noticed that when the spinner appears, the text either shifts to the left or gets overlapped by the spinner, which doesn’t feel quite right. I’d recommend reserving space for the spinner to avoid this issue.

Also, Its not possible to avoid !important at least in this case.

@t-hamano
Copy link
Contributor

I noticed that when the spinner appears, the text either shifts to the left or gets overlapped by the spinner, which doesn’t feel quite right.

This is understandable, but it affects every place the URLInput component is used.

For example, the Social Icon block has space to the right even though the spinner is not visible at all:

image

I would suggest the following changes to the trunk. What do you think?

diff --git a/packages/block-editor/src/components/url-input/style.scss b/packages/block-editor/src/components/url-input/style.scss
index 579b311766..3bc8e68c37 100644
--- a/packages/block-editor/src/components/url-input/style.scss
+++ b/packages/block-editor/src/components/url-input/style.scss
@@ -28,6 +28,10 @@ $input-size: 300px;
                top: calc(50% - #{$spinner-size} / 2);
                right: $grid-unit;
        }
+
+       &:has(.components-spinner) .components-input-control__input {
+               padding-right: $grid-unit-40;
+       }
 }
 
 // Suggestions

@t-hamano t-hamano added the [Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting) label May 23, 2025
@sarthaknagoshe2002
Copy link
Contributor Author

Excellent idea! lets go ahead with your way.

@sarthaknagoshe2002
Copy link
Contributor Author

FYI this approach also gives that bouncy effect when the spinner appears & the text is overlapped by the spinner too.

@t-hamano
Copy link
Contributor

FYI this approach also gives that bouncy effect when the spinner appears & the text is overlapped by the spinner too.

I understand this. At the moment, there doesn't seem to be an ideal solution if we want to keep the spinner.

I'd also like to hear from people who have worked on this component in the past.

cc @getdave @scruffian @WordPress/gutenberg-design


Problem

We are trying to solve the overlap between the text and the spinner.

302344594-3e6db28c-480a-4dd3-bccc-97262f993fbe

First approach (current state of this PR)

Renders right padding conditionally.

Disadvantages: Causes text bouncing each time the spinner changes visibility.

6d9be2dd04b354b54ed62830f083526d.mp4

Second approach

Always reserve the right padding for the spinner.

Disadvantages: Right padding is always applied, even if we don't render a spinner at all.

447009253-e83b1dd8-4cce-4a90-84e7-ba29624e3adf

@jasmussen
Copy link
Contributor

In a sense, the spinner is there to indicate "busy", that the input field is in a state where you have to wait before continuing. Does it make sense in this case to change the line-break indicator to the spinner-state, while this is happening? Since focus could be there we probably can't replace it outright, but potentially that button could just be set to isBusy while working.

@t-hamano
Copy link
Contributor

@jasmussen Thank you for the great feedback!

@sarthaknagoshe2002 Would you like to try the proposed approach?

@sarthaknagoshe2002
Copy link
Contributor Author

I had the same alternative in my mind, I'll re-work on this soon.

@sarthaknagoshe2002
Copy link
Contributor Author

I’ve now set the button to isBusy when the user types, and completely removed the spinner. Let me know if there’s a better way to handle this.

@t-hamano
Copy link
Contributor

I missed an important point. Because the URLInput component is a public component, the suffix doesn't necessarily mean that it's a button component. If we added the isBusy prop to something other than a Button component, we'd get a browser warning or cause unintended problems with that component.

Let's consider a different approach.

@sarthaknagoshe2002
Copy link
Contributor Author

Maybe we could add a check to ensure that isBusy is set only when the component is button & the icon is keyboard return.
What are your thoughts on this?

@t-hamano
Copy link
Contributor

we could add a check to ensure that isBusy is set only when the component is button & the icon is keyboard return.

I don't think this approach is appropriate.

Because if a component that doesn't meet this condition is used as a suffix, there will be no element to represent the loading state, and the user will have no way of knowing if it's loading or not.

Of the two remaining approaches, I tend to prefer the former:

  • Renders right padding conditionally.
  • Always reserve the right padding for the spinner.

In the future, it may be necessary to completely rethink the UI itself.. Quoting from this comment:

Generally, I'm not sure that visually placing extraneous control within an input field is ideal in the first place. The visually appearance os such a placement may look more pleasant but that's subjective and not that relevant.

@sarthaknagoshe2002
Copy link
Contributor Author

Another approach could be to always reserve the right padding for the spinner when there is a suffix. This will ensure that the padding is reserved only for those url-inputs where a suffix is set.

@t-hamano
Copy link
Contributor

Another approach could be to always reserve the right padding for the spinner when there is a suffix.

I don't think there's any relation between providing the suffix prop and whether the spinner is rendered or not.

My understanding is that the spinner may be rendered when both __experimentalShowInitialSuggestions and __experimentalHandleURLSuggestions are not falsy. For example, what do you think of the following change?

Details
diff --git a/packages/block-editor/src/components/url-input/index.js b/packages/block-editor/src/components/url-input/index.js
index 60cbf4d5e8..bfed542739 100644
--- a/packages/block-editor/src/components/url-input/index.js
+++ b/packages/block-editor/src/components/url-input/index.js
@@ -422,6 +422,8 @@ class URLInput extends Component {
                        instanceId,
                        placeholder = __( 'Paste URL or type to search' ),
                        __experimentalRenderControl: renderControl,
+                       __experimentalShowInitialSuggestions: showInitialSuggestions,
+                       __experimentalHandleURLSuggestions: handleURLSuggestions,
                        value = '',
                        hideLabelFromVision = false,
                } = this.props;
@@ -441,6 +443,7 @@ class URLInput extends Component {
                        label,
                        className: clsx( 'block-editor-url-input', className, {
                                'is-full-width': isFullWidth,
+                               'has-spinner': showInitialSuggestions && handleURLSuggestions,
                        } ),
                        hideLabelFromVision,
                };
diff --git a/packages/block-editor/src/components/url-input/style.scss b/packages/block-editor/src/components/url-input/style.scss
index 579b311766..3224fcaf06 100644
--- a/packages/block-editor/src/components/url-input/style.scss
+++ b/packages/block-editor/src/components/url-input/style.scss
@@ -22,6 +22,11 @@ $input-size: 300px;
                }
        }
 
+
+       &.has-spinner .components-input-control__input {
+               padding-right: $grid-unit-40;
+       }
+
        .components-spinner {
                position: absolute;
                margin: 0;

This ensures that navigation links and inline links have padding for the spinner, but the social icon block does not have padding.

@sarthaknagoshe2002
Copy link
Contributor Author

This methods seems to solve all our issues. I think this is ready to be merged!

Thanks @t-hamano!

@t-hamano
Copy link
Contributor

Sorry, I think the conditional statements I suggested were incorrect. In some cases, the right padding is not applied as expected.

In all of the following scenarios, right padding must be applied appropriately:

❌ Button block: Input URL from the block toolbar

Even though the spinner is visible, it has no right padding.

image

✅ Format Library: Inline link

image

✅ Social Link block: Input URL

image

❌ Image block: Input URL from the block toolbar

Even though the spinner is visible, it has no right padding.

image

✅ Image block: Change the current media URL:

image

@sarthaknagoshe2002
Copy link
Contributor Author

For me for some reason the media works fine! I'll need to check for the button block.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting) [Package] Block editor /packages/block-editor [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Input URL fields don't reserve space for the spinner icon
3 participants