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.

Sign up for GitHub

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

Jump to bottom

Move gallery link controls to the block toolbar #62762

Merged
merged 28 commits into from
Jul 30, 2024
Merged

Move gallery link controls to the block toolbar #62762

Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
0e6cc04
Move link dropdown to toolbar in gallery block
akasunil Jun 22, 2024
cac26ab
Update e2e test for gallery block's setting
akasunil Jun 23, 2024
a0c3301
Fix e2e test for gallery block images link setting
akasunil Jun 23, 2024
ebd5a36
Merge branch 'trunk' of personal.github.com:WordPress/gutenberg into …
akasunil Jun 25, 2024
7e6f2c6
Fix unit test
akasunil Jun 25, 2024
5ea3de1
Synced with trunk and resolved conflicts
akasunil Jul 14, 2024
0c7a78b
Synced with trunk
akasunil Jul 20, 2024
ea17d8d
Added ToolbarDropdownMenu component instead of DropdownMenu
akasunil Jul 20, 2024
88b75ac
remove remove extra padding around link icon
akasunil Jul 20, 2024
cd83a3e
Remove unused components
akasunil Jul 20, 2024
6f40925
Revert "Synced with trunk"
akasunil Jul 20, 2024
c795bfa
Merge branch 'trunk' of personal.github.com:WordPress/gutenberg into …
akasunil Jul 20, 2024
694e665
Improve snackbar notices for gallery block link control select
akasunil Jul 22, 2024
8ad541b
Merge branch 'trunk' of personal.github.com:WordPress/gutenberg into …
akasunil Jul 24, 2024
b0f3339
Add constant for lightbox option in link control for gallery block
akasunil Jul 24, 2024
8107157
Update getHrefAndDestination function to handle Link control value ch…
akasunil Jul 24, 2024
eddabc7
Add lightbox option in link control of gallery block
akasunil Jul 24, 2024
3940299
Add info text for lightbox option in link control for gallery block
akasunil Jul 24, 2024
f3e8341
Set info text for lightbox option in single line
akasunil Jul 24, 2024
a321288
Merge branch 'trunk' of personal.github.com:WordPress/gutenberg into …
akasunil Jul 25, 2024
1228c00
reverting expand of click option changes
akasunil Jul 26, 2024
31dcc28
add change to resolve conflict
akasunil Jul 26, 2024
0bf1cac
fix: Avoid unsupported Gallery MenuGroup usage (#63953)
dcalhoun Jul 26, 2024
9456d3d
removing lightbox changes from edit component of gallery block
akasunil Jul 26, 2024
7976411
Removing unneccessory class
akasunil Jul 26, 2024
37a191b
Update link control lable in gallery block
akasunil Jul 27, 2024
893666c
Update label in unit test
akasunil Jul 27, 2024
16f87e7
Update text in snapshot
akasunil Jul 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/block-library/src/gallery/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export const LINK_DESTINATION_NONE = 'none';
export const LINK_DESTINATION_MEDIA = 'media';
export const LINK_DESTINATION_LIGHTBOX = 'lightbox';
export const LINK_DESTINATION_ATTACHMENT = 'attachment';
export const LINK_DESTINATION_MEDIA_WP_CORE = 'file';
export const LINK_DESTINATION_ATTACHMENT_WP_CORE = 'post';
84 changes: 71 additions & 13 deletions packages/block-library/src/gallery/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ import {
ToggleControl,
RangeControl,
Spinner,
MenuGroup,
MenuItem,
ToolbarDropdownMenu,
withNotices,
} from '@wordpress/components';
import {
Expand All @@ -33,6 +36,13 @@ import { View } from '@wordpress/primitives';
import { createBlock } from '@wordpress/blocks';
import { createBlobURL } from '@wordpress/blob';
import { store as noticesStore } from '@wordpress/notices';
import {
link as linkIcon,
customLink,
image as imageIcon,
linkOff,
fullscreen,
} from '@wordpress/icons';

/**
* Internal dependencies
Expand All @@ -49,6 +59,7 @@ import {
LINK_DESTINATION_ATTACHMENT,
LINK_DESTINATION_MEDIA,
LINK_DESTINATION_NONE,
LINK_DESTINATION_LIGHTBOX,
} from './constants';
import useImageSizes from './use-image-sizes';
import useGetNewImages from './use-get-new-images';
Expand All @@ -57,11 +68,30 @@ import GapStyles from './gap-styles';

const MAX_COLUMNS = 8;
const linkOptions = [
{ value: LINK_DESTINATION_ATTACHMENT, label: __( 'Attachment Page' ) },
{ value: LINK_DESTINATION_MEDIA, label: __( 'Media File' ) },
{
value: LINK_DESTINATION_NONE,
icon: customLink,
label: __( 'Link images to attachment pages' ),
value: LINK_DESTINATION_ATTACHMENT,
noticeText: __( 'Attachment Pages' ),
},
{
icon: imageIcon,
label: __( 'Link images to media files' ),
value: LINK_DESTINATION_MEDIA,
noticeText: __( 'Media Files' ),
},
{
icon: fullscreen,
label: __( 'Expand on click' ),
value: LINK_DESTINATION_LIGHTBOX,
noticeText: __( 'Lightbox effect' ),
infoText: __( 'Scales the image with a lightbox effect' ),
},
{
icon: linkOff,
label: _x( 'None', 'Media item link option' ),
value: LINK_DESTINATION_NONE,
noticeText: __( 'None' ),
},
];
const ALLOWED_MEDIA_TYPES = [ 'image' ];
Expand Down Expand Up @@ -359,7 +389,7 @@ function GalleryEdit( props ) {
sprintf(
/* translators: %s: image size settings */
__( 'All gallery image links updated to: %s' ),
linkToText.label
linkToText.noticeText
),
{
id: 'gallery-attributes-linkTo',
Expand Down Expand Up @@ -552,15 +582,6 @@ function GalleryEdit( props ) {
size="__unstable-large"
/>
) }
<SelectControl
__nextHasNoMarginBottom
label={ __( 'Link to' ) }
value={ linkTo }
onChange={ setLinkTo }
options={ linkOptions }
hideCancelButton
size="__unstable-large"
/>
<ToggleControl
__nextHasNoMarginBottom
label={ __( 'Crop images to fit' ) }
Expand Down Expand Up @@ -594,6 +615,43 @@ function GalleryEdit( props ) {
) }
</PanelBody>
</InspectorControls>
<BlockControls group="block">
<ToolbarDropdownMenu
icon={ linkIcon }
label={ __( 'Link To' ) }
>
{ ( { onClose } ) => (
<MenuGroup className="blocks-gallery__link-to-control">
{ linkOptions.map( ( linkItem ) => {
const isOptionSelected =
linkTo === linkItem.value;
return (
<MenuItem
Copy link
Member

@dcalhoun dcalhoun Jul 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@geriux @dcalhoun, If you know why the unit tests are failing, I would appreciate it if you could let me know 🙇‍♂️ This PR moves the link control of the Gallery block from the block sidebar to the block toolbar.

First, thank you for attempting to resolve the mobile unit test failures and for the ping. Much appreciated! 🙇🏻

Both MenuGroup and MenuItem are unavailable for the native mobile editor currently. This results in the crash and unit test failure regarding the Dropdown component attempting to render/return an undefined value.

We encountered this situation before and addressed it in #35191 by bifurcating the code along the web and native platform line. It is not an ideal solution — see #35191 (comment) — but, unfortunately, the lack of support for MenuGroup and MenuItem remains the current state.

If we take a similar approach here, we might retain the previous SelectControl for native platforms. I opened #63953 implementing this option, if we'd like to use this approach to move the proposed web changes forward quickly.

Unfortunately, I do not believe the native mobile team will prioritize MenuGroup or MenuItem components in the near future.

Copy link
Member Author

Choose a reason for hiding this comment

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

@dcalhoun appreciate your response. Thank you for the details on the issue and PR too.

Copy link
Member Author

Choose a reason for hiding this comment

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

@t-hamano is it worth doing this way?

key={ linkItem.value }
isSelected={ isOptionSelected }
className={ clsx(
'components-dropdown-menu__menu-item',
{
'is-active': isOptionSelected,
}
) }
iconPosition="left"
icon={ linkItem.icon }
onClick={ () => {
setLinkTo( linkItem.value );
onClose();
} }
role="menuitemradio"
info={ linkItem.infoText ?? false }
>
{ linkItem.label }
</MenuItem>
);
} ) }
</MenuGroup>
) }
</ToolbarDropdownMenu>
</BlockControls>
{ Platform.isWeb && (
<>
{ ! multiGallerySelection && (
Expand Down
8 changes: 8 additions & 0 deletions packages/block-library/src/gallery/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,3 +199,11 @@
justify-content: flex-end;
gap: $grid-unit-15;
}

.blocks-gallery__link-to-control {
.components-menu-item__item {
.components-menu-item__info {
white-space: nowrap;
}
}
}
9 changes: 4 additions & 5 deletions packages/block-library/src/gallery/test/index.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -610,12 +610,11 @@ describe( 'Gallery block', () => {
<!-- /wp:gallery -->`,
numberOfItems: 2,
} );
const { getByText } = screen;
const { getByLabelText, getByText } = screen;

// Set "Link to" setting via Gallery block settings
await openBlockSettings( screen );
fireEvent.press( getByText( 'Link to' ) );
fireEvent.press( getByText( 'Media File' ) );
fireEvent.press( getBlock( screen, 'Gallery' ) );
fireEvent.press( getByLabelText( 'Link To' ) );
fireEvent.press( getByText( 'Link images to media files' ) );

expect( getEditorHtml() ).toMatchSnapshot();
} );
Expand Down
7 changes: 7 additions & 0 deletions packages/block-library/src/gallery/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
LINK_DESTINATION_NONE,
LINK_DESTINATION_MEDIA_WP_CORE,
LINK_DESTINATION_ATTACHMENT_WP_CORE,
LINK_DESTINATION_LIGHTBOX,
} from './constants';
import {
LINK_DESTINATION_ATTACHMENT as IMAGE_LINK_DESTINATION_ATTACHMENT,
Expand Down Expand Up @@ -43,6 +44,12 @@ export function getHrefAndDestination(
href: image?.link,
linkDestination: IMAGE_LINK_DESTINATION_ATTACHMENT,
};
case LINK_DESTINATION_LIGHTBOX:
return {
href: undefined,
lightbox: { enabled: true },
linkDestination: IMAGE_LINK_DESTINATION_NONE,
};
case LINK_DESTINATION_NONE:
return {
href: undefined,
Expand Down
Loading