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

[DEV-13852] Refactoring - Allow custom rendering of placeholders #574

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -61,16 +61,7 @@ export interface Parameters {
withGalleryPlaceholders?: boolean | { withMediaGalleryTab: WithMediaGalleryTab };
withGalleryBookmarkPlaceholders?:
| false
| Pick<
GalleryBookmarkPlaceholderElement.Props,
| 'fetchOembed'
| 'getSuggestions'
| 'invalidateSuggestions'
| 'renderAddon'
| 'renderEmpty'
| 'renderSuggestion'
| 'renderSuggestionsFooter'
>;
| Pick<GalleryBookmarkPlaceholderElement.Props, 'renderPlaceholder'>;
withImagePlaceholders?:
| boolean
| { withCaptions: boolean; withMediaGalleryTab: WithMediaGalleryTab };
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { ReactNode } from 'react';
import React, { type MouseEvent, useState } from 'react';
import { Transforms } from 'slate';
import { ReactEditor, type RenderElementProps, useSlateStatic } from 'slate-react';
Expand All @@ -14,7 +15,9 @@ import { type Props as BaseProps, Placeholder } from './Placeholder';
export type Props = RenderElementProps &
Pick<BaseProps, 'icon' | 'title' | 'description' | 'format' | 'onClick' | 'onDrop'> & {
element: PlaceholderNode;
overflow?: 'visible' | 'hidden' | 'auto';
removable: RemovableFlagConfig;
renderFrame?: () => ReactNode;
};

export function PlaceholderElement({
Expand All @@ -27,7 +30,9 @@ export function PlaceholderElement({
icon,
title,
description,
overflow,
removable,
renderFrame,
// Callbacks
onClick,
onDrop,
Expand Down Expand Up @@ -63,29 +68,34 @@ export function PlaceholderElement({
<EditorBlock
{...attributes}
element={element}
overflow={overflow}
renderAboveFrame={children}
renderReadOnlyFrame={({ isSelected }) => (
<Placeholder
// Core
active={isActive}
format={format}
icon={icon}
title={title}
description={description}
// Variations
dragOver={onDrop ? dragOver : false}
dropZone={Boolean(onDrop)}
selected={isSelected}
progress={progress ?? isLoading}
// Callbacks
onClick={isLoading ? undefined : onClick}
onRemove={isRemovable ? handleRemove : undefined}
onDragOver={handleDragOver}
onDragLeave={handleDragLeave}
onDrop={onDrop}
onMouseOver={handleMouseOver}
/>
)}
renderReadOnlyFrame={({ isSelected }) =>
renderFrame ? (
renderFrame()
) : (
<Placeholder
// Core
active={isActive}
format={format}
icon={icon}
title={title}
description={description}
// Variations
dragOver={onDrop ? dragOver : false}
dropZone={Boolean(onDrop)}
selected={isSelected}
progress={progress ?? isLoading}
// Callbacks
onClick={isLoading ? undefined : onClick}
onRemove={isRemovable ? handleRemove : undefined}
onDragOver={handleDragOver}
onDragLeave={handleDragLeave}
onDrop={onDrop}
onMouseOver={handleMouseOver}
/>
)
}
rounded
void
/>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,57 +1,51 @@
import type { NewsroomGallery } from '@prezly/sdk';
import type { NewsroomGallery, OEmbedInfo } from '@prezly/sdk';
import type { BookmarkNode } from '@prezly/slate-types';
import React from 'react';
import type { ReactNode } from 'react';
import React, { useEffect, useState } from 'react';
import { Transforms } from 'slate';
import { useSelected, useSlateStatic } from 'slate-react';

import { SearchInput } from '#components';
import { PlaceholderGallery } from '#icons';
import { useFunction } from '#lib';

import { EventsEditor } from '#modules/events';

import { createGalleryBookmark } from '../../gallery-bookmark';
import type { Props as PlaceholderElementProps } from '../components/PlaceholderElement';
import {
type Props as BaseProps,
SearchInputPlaceholderElement,
} from '../components/SearchInputPlaceholderElement';
PlaceholderElement,
type Props as PlaceholderElementProps,
} from '../components/PlaceholderElement';
import { type Props as BaseProps } from '../components/SearchInputPlaceholderElement';
import { replacePlaceholder } from '../lib';
import type { PlaceholderNode } from '../PlaceholderNode';
import { PlaceholdersManager, usePlaceholderManagement } from '../PlaceholdersManager';
import type { FetchOEmbedFn } from '../types';

export function GalleryBookmarkPlaceholderElement({
attributes,
children,
element,
fetchOembed,
format = 'card',
getSuggestions,
removable,
renderAddon,
renderEmpty,
renderSuggestion,
renderSuggestionsFooter,
...props
renderPlaceholder,
}: GalleryBookmarkPlaceholderElement.Props) {
const [isCustomRendered, setCustomRendered] = useState(false);
const editor = useSlateStatic();
const isSelected = useSelected();

const handleTrigger = useFunction(() => {
PlaceholdersManager.activate(element);
});

const handleSelect = useFunction(async (uuid: string, { url }: NewsroomGallery) => {
EventsEditor.dispatchEvent(editor, 'gallery-bookmark-placeholder-submitted', {
gallery: { uuid },
});
const handleSelect = useFunction(
(promise: Promise<{ oembed?: BookmarkNode['oembed']; url: BookmarkNode['url'] }>) => {
setCustomRendered(false);

const loading = fetchOembed(url).then(
(oembed) => ({ oembed, url }),
() => ({ url }), // `oembed` is undefined if an error occurred
);
PlaceholdersManager.register(element.type, element.uuid, promise);
PlaceholdersManager.deactivateAll();
},
);

PlaceholdersManager.register(element.type, element.uuid, loading);
PlaceholdersManager.deactivateAll();
const handleRemove = useFunction(() => {
Transforms.removeNodes(editor, { at: [], match: (node) => node === element });
});

const handleData = useFunction(
Expand All @@ -72,61 +66,45 @@ export function GalleryBookmarkPlaceholderElement({

usePlaceholderManagement(element.type, element.uuid, {
onTrigger: handleTrigger,
// @ts-expect-error Figure out how to fix this
onResolve: handleData,
});

useEffect(() => {
if (!isSelected) {
setCustomRendered(false);
}
}, [isSelected]);

return (
<SearchInputPlaceholderElement<NewsroomGallery>
{...props}
<PlaceholderElement
attributes={attributes}
element={element}
// Core
format={format}
icon={PlaceholderGallery}
title="Click to insert a media gallery bookmark"
description="Add a link to your media gallery"
// Input
getSuggestions={getSuggestions}
renderAddon={renderAddon}
renderEmpty={renderEmpty}
renderSuggestion={renderSuggestion}
renderSuggestions={(props) => (
<SearchInput.Suggestions
activeElement={props.activeElement}
query={props.query}
suggestions={props.suggestions}
footer={renderSuggestionsFooter?.(props)}
origin={props.origin}
>
{props.children}
</SearchInput.Suggestions>
)}
inputTitle="Media gallery bookmark"
inputDescription="Add a media gallery card to your stories, campaigns and pitches"
inputPlaceholder="Search for media galleries"
onSelect={handleSelect}
onClick={() => setCustomRendered(true)}
overflow="visible"
renderFrame={
isCustomRendered
? () => renderPlaceholder({ onRemove: handleRemove, onSelect: handleSelect })
: undefined
}
removable={removable}
Copy link
Contributor Author

@kudlajz kudlajz Nov 5, 2024

Choose a reason for hiding this comment

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

I am still wondering if it's a good idea to have this half-approach where the "initial" placeholder UI is rendered by the editor, but then the subsequent UI is custom rendered.

I think ideally it would be nice if we could render the whole placeholder outside of the editor (i.e. rendered via renderPlaceholder function) but this would also mean that we would need to implement drag & drop functionality and other things (loading states, placeholder management etc.), so I wasn't sure if we should do it now or at all.

EDIT: We probably don't need to worry about placeholder management outside of the editor. It would still be internally managed by the editor as needed when using the respective callbacks.

The advantage of this "headless" approach is that we would have a complete control over the UI of the placeholders within the app and it will also allow other developers to bring their own UI (currently we force the placeholders to look the way they do, which I don't think is great).

Just wanted to hear your thoughts and opinions before I do the work, in case you don't think it's a good idea.

What do you think?

Copy link
Contributor

@e1himself e1himself Nov 6, 2024

Choose a reason for hiding this comment

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

I generally dislike how these extensions are structured with the code split between the app repo and the editor repo. Making the extension hard-to-use outside of the Prezly app. But hey! One step at a time! :)

With this refactoring I can definitely see that things become less complicated. And even though the code splitting issue is still there, it is a great improvement 👍

>
{children}
</SearchInputPlaceholderElement>
</PlaceholderElement>
);
}

export namespace GalleryBookmarkPlaceholderElement {
export interface Props
extends Omit<
BaseProps<NewsroomGallery>,
| 'onSelect'
| 'icon'
| 'title'
| 'description'
| 'inputTitle'
| 'inputDescription'
| 'inputPlaceholder'
| 'renderSuggestions'
>,
extends Pick<BaseProps<NewsroomGallery>, 'attributes' | 'children' | 'element' | 'format'>,
Pick<PlaceholderElementProps, 'removable'> {
element: PlaceholderNode<PlaceholderNode.Type.GALLERY_BOOKMARK>;
fetchOembed: FetchOEmbedFn;
renderSuggestionsFooter?: BaseProps<NewsroomGallery>['renderSuggestions'];
renderPlaceholder: (props: {
onRemove: () => void;
onSelect: (promise: Promise<{ oembed?: OEmbedInfo; url: string }>) => void;
}) => ReactNode;
}
}
3 changes: 1 addition & 2 deletions packages/slate-editor/src/modules/editor/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,7 @@ export function getAllExtensions() {
withFloatingAddMenu: true,
withGalleries: {},
withGalleryBookmarks: {
fetchOembed,
getSuggestions: () => [],
renderPlaceholder: () => null,
},
withHeadings: true,
withImages: {
Expand Down
11 changes: 1 addition & 10 deletions packages/slate-editor/src/modules/events/types.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,5 @@
import type { Listener } from '@prezly/events';
import type {
CoverageEntry,
NewsroomContact,
NewsroomGallery,
OEmbedInfo,
Story,
} from '@prezly/sdk';
import type { CoverageEntry, NewsroomContact, OEmbedInfo, Story } from '@prezly/sdk';
import type {
GalleryImageSize,
GalleryLayout,
Expand Down Expand Up @@ -110,9 +104,6 @@ export type EditorEventMap = {
'gallery-layout-changed': {
layout: GalleryLayout;
};
'gallery-bookmark-placeholder-submitted': {
gallery: Pick<NewsroomGallery, 'uuid'>;
};
'image-added': {
description: string;
isPasted: boolean;
Expand Down
Loading