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.

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

Block Editor: LinkControl: Add Submit button as action #19971

Closed
wants to merge 4 commits into from

Conversation

aduth
Copy link
Member

@aduth aduth commented Jan 30, 2020

Extracted from #19462
Related: #19570

This pull request seeks to add a new "Submit" button to be shown alongside instead of the existing "Reset" button in the new LinkControl component used by the Navigation and Buttons blocks.

Before After
Before After
Original Before/After
Before After
Before After

Rationale

It only really occurs to me now, but I'm not really sure what benefits this brings.

If I had to guess:

  • It could make it visually clearer that this is a form to be submitted, whereas otherwise it relies on the user having knowledge that pressing Enter is an equally effective means to submit the form.
  • According to the original commit 90f7b5d from Try the link control in the link format #19462, "When clicking Change on an existing link you should be able to modify and then resubmit the link without having to repeat your search over again.". It's not really clear to me what impact there is on this behavior in whether or not a Submit button is present, though this could have been more relevant prior to changes in the form submission behavior as in Block Editor: Handle LinkControl submission via form handler #19651 (cc @getdave).

Even if we decide not to move forward with adding a submit button, and depending on some decision out of #19570 as to whether the "Reset" button should be removed as well, the proposed changes could still be valuable in providing a more reliable computation of offsets for absolute positioning of the spinner and reset buttons.

Testing Instructions:

Verify that when editing a link in a Navigation or Buttons block, you should see a Submit button shown an activated as long as the input contains a valid link.

Check as well that issues raised in #19462 with the original implementation have been addressed:

@shaunandrews
Copy link
Contributor

It only really occurs to me now, but I'm not really sure what benefits this brings.

I couldn't have said it better myself. The only thing that comes to mind is that a visual submit button might be more obvious and accessible than using the return key to trigger the action.

I will say, having two actions within the input is probably too much. If we decide to add a submit button, then perhaps we should remove the reset button to keep the number of actions to one.

@aduth
Copy link
Member Author

aduth commented Jan 31, 2020

The only thing that comes to mind is that a visual submit button might be more obvious and accessible than using the return key to trigger the action.

Yeah, personally, I would prefer that anything which behaves like a form (takes a submission) would have some explicit Submit button somewhere. I also recognize that it's somewhat at odds with the otherwise minimalistic aesthetic of the field.

I will say, having two actions within the input is probably too much.

I mentioned in passing in the original comment, but #19570 could be very much relevant to this conversation. That is, f we decide that the "Reset" button should be removed, could it be reasonable then to keep the "Submit" button (i.e. swap out Reset for Submit).

@aduth
Copy link
Member Author

aduth commented Feb 3, 2020

The issue I opened at #20007 has relevance, since it explores what might be necessary to reintroduce a "Link Settings" button/toggle to the editing state of this component. That would be a third option, in addition to the existing "Submit" and "Reset" considerations. What I propose there is to remove "Reset" altogether, and go with "Submit" and "Settings". That's still two actions, but it is consistent with how this UI has existed up 'til now (the previous Gutenberg link UI and the link UI in the Classic Editor).

At the very least, to keep this moving, I might still like to implement replacing "Reset" with just "Submit", but including all of the CSS refactoring which should make it simpler to adjust these buttons in the future (adding, removing, or substituting).

aduth added 4 commits February 3, 2020 15:46
Allow for reuse in computing dimensions in other stylesheets
Instead inherit that the form cannot be submitted since the rendered URLInput will apply a `required` attribute, preventing submission of the form, while still displaying the associated help text "You must fill out this field"
@aduth aduth force-pushed the update/link-control-submit-button branch from 67b86c9 to 62ff4a5 Compare February 3, 2020 23:46
@aduth
Copy link
Member Author

aduth commented Feb 3, 2020

I've rebased the branch to resolve conflicts, and per my previous comment, to change the purpose of the pull request to replace the "Reset" button with the Submit button. I've also altered the implementation slightly to further consolidate the dimensions calculations as based on the number of actions present (anticipating that this could potentially change in the future).

@getdave
Copy link
Contributor

getdave commented Feb 5, 2020

I agree with adding a button to allow submission.

I think "RESET" is probably an edge case and not a primary action.

Therefore, if we need to remove one to save space and reduce UI clutter (and potential choice paralysis) then I vote to remove Reset.

@aduth
Copy link
Member Author

aduth commented Feb 7, 2020

Noting that this was cherry-picked to be absorbed into the efforts at #20052, though this one could still be landed separately as more targeted to the specific change.

@aduth
Copy link
Member Author

aduth commented Feb 10, 2020

Closing, since #20052 included the changes proposed here.

@aduth aduth closed this Feb 10, 2020
@aduth aduth deleted the update/link-control-submit-button branch February 10, 2020 15:57
@aduth aduth added the [Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting) label Feb 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Affects the Navigation Block [Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting) [Package] Block editor /packages/block-editor [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants