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

Add inline media controls for the Image component #269

Merged
merged 14 commits into from
Jan 29, 2024
Merged

Add inline media controls for the Image component #269

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
905324e
Added inline image controls options
ncoetzer Oct 6, 2023
e92b5ce
Refactored the inline controls as a separate component
ncoetzer Oct 6, 2023
a7225b1
Changes Image.Image component to Media and split into separate compon…
ncoetzer Oct 6, 2023
4491c43
Refactored the index to pull in external components
ncoetzer Oct 6, 2023
4cb8b51
Split the figure component out into its own file
ncoetzer Oct 6, 2023
9126341
Refactored child components into their own sub directory for easier i…
ncoetzer Oct 6, 2023
e670cea
Updated readme
ncoetzer Oct 6, 2023
4dba3d1
Fixes media styles block error
ncoetzer Oct 6, 2023
25394d6
Reworked the style prop for rendering conditional
ncoetzer Oct 6, 2023
5a64b45
Updated figure styles
ncoetzer Oct 6, 2023
f006e22
Updated figire component style prop
ncoetzer Oct 6, 2023
4db07f2
Updated media component props
ncoetzer Oct 6, 2023
786c680
Created `Figure` compound component for the main Image component
ncoetzer Oct 6, 2023
df92c33
Added styling fix for the figure lement for block API 3
ncoetzer Oct 6, 2023
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
25 changes: 25 additions & 0 deletions components/image/child-components/figure.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import PropTypes from 'prop-types';
import { StyledComponentContext } from '../../styled-components-context';
import { InlineControlsStyleWrapper } from '../styles';

export const Figure = (props) => {
const { style, children, ...rest } = props;

return (
<StyledComponentContext cacheKey="tenup-component-image">
<InlineControlsStyleWrapper {...style} {...rest}>
Copy link
Member

Choose a reason for hiding this comment

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

Would this also work as a singling if the image tag?

I'd love to not add an additional div around the image :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fabiankaegy Think this is where I got confused and am not sure I'm following 😆. Apologies! So, the Figure component already had the figure wrapper before the change:

const Figure = (props) => {
	const { children, style, ...rest } = props;

	return (
		<figure style={{ position: 'relative', ...style }} {...rest}>
			{children}
		</figure>
	);
};

I basically just swapped it for a styled component that also renders a figure.

Should there not be other conditional log in the main component? Or am I missing something completely.

Copy link
Member

Choose a reason for hiding this comment

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

@ncoetzer Okay I may have misunderstood the code then.

What I was trying to get at is that the end markup should look like this:

<figure>
    <img />
    <div>
        <!-- Insert anything else here -->
    </div>
</figure>

and not:

<figure>
    <div>
         <img />
        <!-- Insert anything else here -->
    </div>
</figure>

Copy link
Contributor Author

@ncoetzer ncoetzer Oct 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@fabiankaegy Aah okay cool. In that case, yes, that is how it currently works and outputs the code.
image

However, for any existing instances of this component, it would also wrap their <img> tags inside a <figure>, which is a breaking change if I understood things correctly. So, I'm thinking of changing the logic in the main component to conditionally check if inline controls are needed, and if so, wrap the <img> in a <figure>. Otherwise, it will just output the <img> directly as it currently does. We could do this with any other feature placed behind a flag as well.

Of course, this new API also accommodates creating custom children, so the user would be able to override anything we have with anything they need.

Thoughts?

{children}
</InlineControlsStyleWrapper>
</StyledComponentContext>
);
};

Figure.defaultProps = {
style: {},
children: [],
};

Figure.propTypes = {
style: PropTypes.object,
children: PropTypes.array,
};
5 changes: 5 additions & 0 deletions components/image/child-components/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { Media, ImageContext } from './media';
import { Figure } from './figure';
import { InlineControls } from './inline-controls';

export { Media, ImageContext, Figure, InlineControls };
39 changes: 39 additions & 0 deletions components/image/child-components/inline-controls.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { __ } from '@wordpress/i18n';
import { MediaReplaceFlow } from '@wordpress/block-editor';
import { ToolbarButton } from '@wordpress/components';
import PropTypes from 'prop-types';

/**
* Internal Dependencies
*/

export const InlineControls = (props) => {
const { imageUrl, onSelect, isOptional, onRemove } = props;

return (
<div className="inline-controls-sticky-wrapper">
<div className="inline-controls">
<MediaReplaceFlow mediaUrl={imageUrl} onSelect={onSelect} name={__('Replace')} />
{!!isOptional && (
<ToolbarButton onClick={onRemove} className="remove-button">
{__('Remove')}
</ToolbarButton>
)}
</div>
</div>
);
};

InlineControls.defaultProps = {
imageUrl: '',
onSelect: undefined,
isOptional: false,
onRemove: undefined,
};

InlineControls.propTypes = {
imageUrl: PropTypes.string,
onSelect: PropTypes.func,
isOptional: PropTypes.bool,
onRemove: PropTypes.func,
};
78 changes: 78 additions & 0 deletions components/image/child-components/media.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import PropTypes from 'prop-types';
import { MediaPlaceholder, InspectorControls } from '@wordpress/block-editor';
import { useContext, createContext } from '@wordpress/element';
import { Spinner, FocalPointPicker, PanelBody, Placeholder } from '@wordpress/components';
import { __ } from '@wordpress/i18n';

export const ImageContext = createContext();

export const Media = (props) => {
const { style } = props;
const {
imageUrl,
altText,
labels,
onSelect,
isResolvingMedia,
shouldDisplayFocalPointPicker,
focalPoint,
onChangeFocalPoint,
canEditImage,
hasImage,
} = useContext(ImageContext);

if (shouldDisplayFocalPointPicker) {
const focalPointStyle = {
objectFit: 'cover',
objectPosition: `${focalPoint.x * 100}% ${focalPoint.y * 100}%`,
};

props.styles = {
...style,
...focalPointStyle,
};
}

if (isResolvingMedia) {
return <Spinner />;
}

if (!hasImage && !canEditImage) {
return <Placeholder className="block-editor-media-placeholder" withIllustration />;
}

return (
<>
{shouldDisplayFocalPointPicker && (
<InspectorControls>
<PanelBody title={__('Image Settings')}>
<FocalPointPicker
label={__('Focal Point Picker')}
url={imageUrl}
value={focalPoint}
onChange={onChangeFocalPoint}
/>
</PanelBody>
</InspectorControls>
)}
{hasImage && <img src={imageUrl} alt={altText} {...props} />}
{canEditImage && (
<MediaPlaceholder
labels={labels}
onSelect={onSelect}
accept="image"
multiple={false}
disableMediaButtons={imageUrl}
/>
)}
</>
);
};

Media.defaultProps = {
style: {},
};

Media.propTypes = {
style: PropTypes.object,
};
115 changes: 35 additions & 80 deletions components/image/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { MediaPlaceholder, InspectorControls } from '@wordpress/block-editor';
import { useContext, useMemo, Children, createContext } from '@wordpress/element';
import { Spinner, FocalPointPicker, PanelBody, Placeholder } from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import { MediaPlaceholder } from '@wordpress/block-editor';
import { useMemo, Children } from '@wordpress/element';
import PropTypes from 'prop-types';

/**
* Internal Dependencies
*/
import { Media, ImageContext, Figure, InlineControls } from './child-components';
import { useMedia } from '../../hooks/use-media';

export const ImageContext = createContext();

const ImageWrapper = (props) => {
const {
id,
Expand All @@ -18,6 +18,9 @@ const ImageWrapper = (props) => {
labels = {},
canEditImage = true,
children,
hasInlineControls = false,
isOptional = true,
onRemove,
...rest
} = props;
const hasImage = !!id;
Expand Down Expand Up @@ -45,6 +48,9 @@ const ImageWrapper = (props) => {
isResolvingMedia,
shouldDisplayFocalPointPicker,
hasImage,
hasInlineControls,
isOptional,
onRemove,
};
}, [
id,
Expand All @@ -59,6 +65,9 @@ const ImageWrapper = (props) => {
isResolvingMedia,
shouldDisplayFocalPointPicker,
hasImage,
hasInlineControls,
isOptional,
onRemove,
]);

if (hasRenderCallback) {
Expand All @@ -70,6 +79,9 @@ const ImageWrapper = (props) => {
labels,
canEditImage,
onSelect,
hasInlineControls,
isOptional,
onRemove,
});
}

Expand All @@ -92,7 +104,15 @@ const ImageWrapper = (props) => {
return (
<ImageContext.Provider value={imageContext}>
<Figure>
<Image {...rest} />
<Media {...rest} />
{hasImage && !!hasInlineControls && (
<InlineControls
imageUrl={imageUrl}
onSelect={onSelect}
isOptional={isOptional}
onRemove={onRemove}
/>
)}
</Figure>
</ImageContext.Provider>
);
Expand All @@ -106,6 +126,10 @@ ImageWrapper.defaultProps = {
onChangeFocalPoint: undefined,
labels: {},
canEditImage: true,
hasInlineControls: false,
isOptional: true,
onRemove: undefined,
children: [],
};

ImageWrapper.propTypes = {
Expand All @@ -122,77 +146,8 @@ ImageWrapper.propTypes = {
instructions: PropTypes.string,
}),
canEditImage: PropTypes.bool,
};

const Figure = (props) => {
const { children, style, ...rest } = props;

return (
<figure style={{ position: 'relative', ...style }} {...rest}>
{children}
</figure>
);
};

const Image = (props) => {
const { style } = props;
const {
imageUrl,
altText,
labels,
onSelect,
isResolvingMedia,
shouldDisplayFocalPointPicker,
focalPoint,
onChangeFocalPoint,
canEditImage,
hasImage,
} = useContext(ImageContext);

if (shouldDisplayFocalPointPicker) {
const focalPointStyle = {
objectFit: 'cover',
objectPosition: `${focalPoint.x * 100}% ${focalPoint.y * 100}%`,
};

props.style = {
...style,
...focalPointStyle,
};
}

if (isResolvingMedia) {
return <Spinner />;
}

if (!hasImage && !canEditImage) {
return <Placeholder className="block-editor-media-placeholder" withIllustration />;
}

return (
<>
{shouldDisplayFocalPointPicker && (
<InspectorControls>
<PanelBody title={__('Image Settings')}>
<FocalPointPicker
label={__('Focal Point Picker')}
url={imageUrl}
value={focalPoint}
onChange={onChangeFocalPoint}
/>
</PanelBody>
</InspectorControls>
)}
{hasImage && <img src={imageUrl} alt={altText} {...props} />}
{canEditImage && (
<MediaPlaceholder
labels={labels}
onSelect={onSelect}
accept="image"
multiple={false}
disableMediaButtons={imageUrl}
/>
)}
</>
);
hasInlineControls: PropTypes.bool,
isOptional: PropTypes.bool,
onRemove: PropTypes.func,
children: PropTypes.array,
};
Loading