Skip to content

Commit ba319c8

Browse files
stokesmant-hamanoMamadukatalldan
authored
Button: Only open the link popover on explicit user action (#30166)
* Button block: hide link popover if not in visual edit mode * Don't open link popover when button is selected * Restore link keyboard shortcut * Update e2e test for Buttons block focus return * Update e2e test for Pattern override with Button block * Restore some functionality * Use platform-agnostic shortcut in Pattern Overrides e2e test Co-Authored-By: Claude <noreply@anthropic.com> * Add changelog entry for Button link popover behavior change Co-Authored-By: Claude <noreply@anthropic.com> * Open link popover before editing bound URL in block bindings e2e test Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: stokesman <presstoke@git.wordpress.org> Co-authored-by: t-hamano <wildworks@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: talldan <talldanwp@git.wordpress.org>
1 parent 177af80 commit ba319c8

5 files changed

Lines changed: 59 additions & 61 deletions

File tree

‎packages/block-library/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
- Media & Text: Lower the specificity of the content area's default padding so themes can override it, including through the block's custom CSS in `theme.json` ([#83563](https://github.com/WordPress/gutenberg/pull/83563)).
1111
- Gallery: Add an "Order by" sorting control to the static gallery mode, and consolidate sorting control for static and dynamic modes in the Settings panel ([#83438](https://github.com/WordPress/gutenberg/pull/83438)).
1212
- Icon: Match an icon's keywords as well as its name and label when searching the icon library ([#82367](https://github.com/WordPress/gutenberg/pull/82367)).
13+
- Button: Only open the link popover from the toolbar button or keyboard shortcut instead of whenever a linked button is selected. The toolbar button now shows a pressed state when a link is set ([#30166](https://github.com/WordPress/gutenberg/pull/30166)).
1314

1415
### Bug Fixes
1516

‎packages/block-library/src/button/edit.jsx‎

Lines changed: 43 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import {
2727
privateApis as blockEditorPrivateApis,
2828
} from '@wordpress/block-editor';
2929
import { displayShortcut, isKeyboardEvent, ENTER } from '@wordpress/keycodes';
30-
import { link, linkOff } from '@wordpress/icons';
30+
import { link } from '@wordpress/icons';
3131
import {
3232
createBlock,
3333
cloneBlock,
@@ -377,64 +377,55 @@ function ButtonEdit( props ) {
377377
{ isLinkTag && ! lockUrlControls && (
378378
<ToolbarButton
379379
name="link"
380-
icon={ ! isURLSet ? link : linkOff }
381-
title={ ! isURLSet ? __( 'Link' ) : __( 'Unlink' ) }
382-
shortcut={
383-
! isURLSet
384-
? displayShortcut.primary( 'k' )
385-
: displayShortcut.primaryShift( 'k' )
386-
}
387-
onClick={ ! isURLSet ? startEditing : unlink }
380+
icon={ link }
381+
title={ __( 'Link' ) }
382+
shortcut={ displayShortcut.primary( 'k' ) }
383+
onClick={ () => setIsEditingURL( ! isEditingURL ) }
388384
isActive={ isURLSet }
389385
/>
390386
) }
391387
</BlockControls>
392388
) }
393-
{ isLinkTag &&
394-
isSelected &&
395-
( isEditingURL || isURLSet ) &&
396-
! lockUrlControls && (
397-
<Popover
398-
placement="bottom"
399-
onClose={ () => {
400-
setIsEditingURL( false );
389+
{ isLinkTag && isSelected && isEditingURL && ! lockUrlControls && (
390+
<Popover
391+
placement="bottom"
392+
onClose={ () => {
393+
setIsEditingURL( false );
394+
richTextRef.current?.focus();
395+
} }
396+
anchor={ popoverAnchor }
397+
focusOnMount={ isEditingURL ? 'firstElement' : false }
398+
__unstableSlotName="__unstable-block-tools-after"
399+
shift
400+
>
401+
<LinkControl
402+
value={ linkValue }
403+
onChange={ ( {
404+
url: newURL,
405+
opensInNewTab: newOpensInNewTab,
406+
nofollow: newNofollow,
407+
} ) =>
408+
setAttributes(
409+
getUpdatedLinkAttributes( {
410+
rel,
411+
url: newURL,
412+
opensInNewTab: newOpensInNewTab,
413+
nofollow: newNofollow,
414+
} )
415+
)
416+
}
417+
onRemove={ () => {
418+
unlink();
401419
richTextRef.current?.focus();
402420
} }
403-
anchor={ popoverAnchor }
404-
focusOnMount={ isEditingURL ? 'firstElement' : false }
405-
__unstableSlotName="__unstable-block-tools-after"
406-
shift
407-
>
408-
<LinkControl
409-
value={ linkValue }
410-
onChange={ ( {
411-
url: newURL,
412-
opensInNewTab: newOpensInNewTab,
413-
nofollow: newNofollow,
414-
} ) =>
415-
setAttributes(
416-
getUpdatedLinkAttributes( {
417-
rel,
418-
url: newURL,
419-
opensInNewTab: newOpensInNewTab,
420-
nofollow: newNofollow,
421-
} )
422-
)
423-
}
424-
onRemove={ () => {
425-
unlink();
426-
richTextRef.current?.focus();
427-
} }
428-
forceIsEditingLink={ isEditingURL }
429-
settings={ LINK_SETTINGS }
430-
createSuggestion={
431-
createPageEntity && handleCreate
432-
}
433-
withCreateSuggestion={ userCanCreatePages }
434-
createSuggestionButtonText={ createButtonText }
435-
/>
436-
</Popover>
437-
) }
421+
forceIsEditingLink={ ! isURLSet }
422+
settings={ LINK_SETTINGS }
423+
createSuggestion={ createPageEntity && handleCreate }
424+
withCreateSuggestion={ userCanCreatePages }
425+
createSuggestionButtonText={ createButtonText }
426+
/>
427+
</Popover>
428+
) }
438429
<InspectorControls group="advanced">
439430
<HTMLElementControl
440431
tagName={ tagName }

‎test/e2e/specs/editor/blocks/buttons.spec.js‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -136,11 +136,6 @@ test.describe( 'Buttons', () => {
136136
await expect(
137137
editor.canvas.locator( 'role=textbox[name="Button text"i]' )
138138
).toBeFocused();
139-
140-
// The link control should still be visible when a URL is set.
141-
await expect(
142-
page.locator( 'role=link[name=/^example\\.com/]' )
143-
).toBeVisible();
144139
} );
145140

146141
test( 'appends https protocol to links added which are missing a protocol', async ( {

‎test/e2e/specs/editor/various/block-bindings/custom-sources.spec.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -900,6 +900,7 @@ test.describe( 'Registered sources', () => {
900900
} )
901901
.getByRole( 'textbox' );
902902
await buttonBlock.click();
903+
await pageUtils.pressKeys( 'primary+k' );
903904
await page
904905
.getByRole( 'button', { name: 'Edit link', exact: true } )
905906
.click();

‎test/e2e/specs/editor/various/pattern-overrides.spec.js‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -792,6 +792,7 @@ test.describe( 'Pattern Overrides', () => {
792792
requestUtils,
793793
editor,
794794
context,
795+
pageUtils,
795796
} ) => {
796797
const buttonName = 'Editable button';
797798
const { id } = await requestUtils.createBlock( {
@@ -811,11 +812,12 @@ test.describe( 'Pattern Overrides', () => {
811812
attributes: { ref: id },
812813
} );
813814

814-
// Focus the button, open the link popup.
815-
await editor.canvas
815+
const button = editor.canvas
816816
.getByRole( 'document', { name: 'Block: Button' } )
817-
.getByRole( 'textbox', { name: 'Button text' } )
818-
.focus();
817+
.getByRole( 'textbox', { name: 'Button text' } );
818+
// Focus the button, open the link popup.
819+
await button.focus();
820+
await pageUtils.pressKeys( 'primary+k' );
819821
await expect(
820822
page.getByRole( 'link', { name: 'wp.org' } ).getByText( '↗' )
821823
).toHaveAttribute( 'aria-label', '(opens in a new tab)' );
@@ -864,6 +866,10 @@ test.describe( 'Pattern Overrides', () => {
864866
await expect( buttonLink ).toHaveAttribute( 'target', '_blank' );
865867
await expect( buttonLink ).toHaveAttribute( 'rel', 'noopener' );
866868

869+
// Reopen the link popup.
870+
await button.focus();
871+
await pageUtils.pressKeys( 'primary+k' );
872+
867873
// Uncheck both checkboxes.
868874
await editLinkButton.click();
869875
await openInNewTabCheckbox.setChecked( false );
@@ -880,6 +886,10 @@ test.describe( 'Pattern Overrides', () => {
880886
await expect( buttonLink ).toHaveAttribute( 'target', '' );
881887
await expect( buttonLink ).toHaveAttribute( 'rel', '' );
882888

889+
// Reopen the link popup.
890+
await button.focus();
891+
await pageUtils.pressKeys( 'primary+k' );
892+
883893
// Check only the "mark as nofollow" checkbox.
884894
await editLinkButton.click();
885895
await markAsNoFollowCheckbox.setChecked( true );

0 commit comments

Comments
 (0)