Skip to content

Commit e75e466

Browse files
committed
TASK: Simplify Link Editor framework by removing first class support for asynchronous models
There is a lot of complexity to async `convertLinkToModel` because we cannot initially merge this value into the linkTypes observeable state -> this was not a problem as all `Editor` work with a nullable `State` either way as its would also be null if not loaded. But we also want to open the additional link options if the link has `anchor` set for example. For that we need the resolved link model see linktype.isValid and we were only be able to compute this one tick later. That creates glitches and content shifts for the opening animation of the dialog. Technically we could solve that by strict separating `DialogWithValue` into `DialogWithValue` and `DialogWithAsyncValue` where the latter would receive a promise and use `usePromise` to resolve that. Splitting it this way would allow non async values to appear immediately which is the default case and async values to be displayed after a loading animation. Now as the core does not need any async link editors we dont need to add this complexity right now just to carry it around. In case a plugin wants to register a dialog with async state it would need to simplify manage that itself.
1 parent ad51d16 commit e75e466

File tree

10 files changed

+97
-186
lines changed

10 files changed

+97
-186
lines changed

packages/framework-promise-react/src/PromiseState.ts

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,3 @@ export type IPromiseState<R> =
33
| {isLoading: true, error: null, value: null}
44
| {isLoading: false, error: Error, value: null}
55
| {isLoading: false, error: null, value: R};
6-
7-
const LOADING: IPromiseState<any> = {isLoading: true, error: null, value: null};
8-
9-
export function forLoading(): IPromiseState<any> {
10-
return LOADING;
11-
}
12-
13-
export function forError(error: Error): IPromiseState<any> {
14-
return {isLoading: false, error, value: null};
15-
}
16-
17-
export function forValue<R>(value: R): IPromiseState<R> {
18-
return {isLoading: false, error: null, value};
19-
}

packages/neos-ui-link-editor-core/src/application/Dialog/Dialog.tsx

Lines changed: 56 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import * as React from 'react';
22

33
import {Button, Tabs, Dialog, Icon} from '@neos-project/react-ui-components';
44

5-
import {ErrorBoundary, ErrorView} from '@neos-project/neos-ui-error';
5+
import {AnyError, ErrorBoundary, ErrorView} from '@neos-project/neos-ui-error';
66
import style from './style.module.css';
77

88
import {
@@ -56,12 +56,28 @@ const ActiveLinkEditorDialog: React.FC<{
5656

5757
const availableLinkTypes = useSortedAndFilteredLinkTypes(editor);
5858

59+
const {error: initialError, model: initialModel} = React.useMemo(() => {
60+
if (!initialLinkType || !initialValue) {
61+
return {error: null, model: null};
62+
}
63+
try {
64+
const model = initialLinkType.convertLinkToModel(initialValue);
65+
return {error: null, model};
66+
} catch (error) {
67+
return {error: error as AnyError, model: null};
68+
}
69+
}, [initialLinkType, initialValue]);
70+
5971
const form$ = React.useMemo(() => createState({
6072
isOptionsDirty: false,
6173
initialLinkWasDeleted: false,
6274
activeLinkTypeId: initialLinkType?.id ?? availableLinkTypes[0].id,
6375
options: initialValue?.options ?? {},
64-
linkModels: {}
76+
linkModels: initialLinkType
77+
? {
78+
[initialLinkType.id]: initialModel
79+
}
80+
: {}
6581
} as FormValues), []);
6682

6783
const formStatus$ = React.useMemo(() => mapState(form$, (form) => {
@@ -152,7 +168,8 @@ const ActiveLinkEditorDialog: React.FC<{
152168
<DialogWithValue
153169
form$={form$}
154170
editor={editor}
155-
initialValue={initialValue}
171+
initialModel={initialModel}
172+
initialError={initialError}
156173
initialLinkType={initialLinkType}
157174
availableLinkTypes={availableLinkTypes}
158175
/>
@@ -222,36 +239,20 @@ const DialogWithEmptyValue: React.FC<{
222239
const DialogWithValue: React.FC<{
223240
form$: State<FormValues>
224241
editor: IEditor,
225-
initialValue: ILink,
242+
initialModel: any,
243+
initialError: AnyError | null
226244
initialLinkType: ILinkType,
227245
availableLinkTypes: ReadonlyArray<ILinkType>
228246
}> = props => {
229247
const {editorOptions} = useLatestState(props.editor.state$);
230-
const {isLoading, error, value: initialModel} = props.initialLinkType.useResolvedModel(props.initialValue);
231248

232249
const setActiveTab = React.useCallback((linkId) => props.form$.update((values) => ({...values, activeLinkTypeId: linkId})), []);
233250
const activeTab$ = React.useMemo(() => mapState(props.form$, (form) => form.activeLinkTypeId), []);
234251
const activeTab = useLatestState(activeTab$);
235252

236-
React.useEffect(() => {
237-
if (initialModel !== null) {
238-
if (!props.form$.current.linkModels[props.initialLinkType.id]) {
239-
// update state with initial value once available
240-
props.form$.update((form) => ({
241-
...form,
242-
linkModels: {
243-
...form.linkModels,
244-
[props.initialLinkType.id]: initialModel
245-
}
246-
}));
247-
}
248-
}
249-
}, [initialModel]);
250-
251253
const linkModels$ = React.useMemo(() => pickState(props.form$, 'linkModels'), []);
252254

253255
const InitialPreview = props.initialLinkType.Preview;
254-
const InitialLoadingPreview = props.initialLinkType.LoadingPreview;
255256

256257
return (
257258
<>
@@ -261,20 +262,13 @@ const DialogWithValue: React.FC<{
261262
availableLinkTypes={props.availableLinkTypes}
262263
form$={props.form$}
263264
initialLinkTypePreview={() => (
264-
error ? (
265-
<ErrorView error={error}/>
265+
props.initialError ? (
266+
<ErrorView error={props.initialError}/>
266267
) : (
267-
isLoading ? (
268-
<InitialLoadingPreview
269-
link={props.initialValue}
270-
options={editorOptions.linkTypes?.[props.initialLinkType.id] as any ?? {}}
271-
/>
272-
) : (
273-
<InitialPreview
274-
model={initialModel}
275-
options={editorOptions.linkTypes?.[props.initialLinkType.id] as any ?? {}}
276-
/>
277-
)
268+
<InitialPreview
269+
model={props.initialModel}
270+
options={editorOptions.linkTypes?.[props.initialLinkType.id] as any ?? {}}
271+
/>
278272
)
279273
)}
280274
/>
@@ -284,7 +278,7 @@ const DialogWithValue: React.FC<{
284278
vertical
285279
>
286280
{props.availableLinkTypes.map((linkType) => {
287-
const {Editor, LoadingEditor} = linkType;
281+
const {Editor} = linkType;
288282
const model$ = React.useMemo(() => pickState(linkModels$, linkType.id), [linkModels$])
289283

290284
return (
@@ -297,27 +291,18 @@ const DialogWithValue: React.FC<{
297291
>
298292
<Layout.Stack>
299293
<ErrorBoundary errorFallback={ErrorView}>
300-
{isLoading && linkType.id === props.initialLinkType.id ? (
301-
<LoadingEditor
302-
link={props.initialValue}
303-
options={editorOptions.linkTypes?.[props.initialLinkType.id] as any ?? {}}
304-
/>
305-
) : (
306-
<>
307-
<Editor
308-
model$={model$}
309-
options={editorOptions.linkTypes?.[linkType.id] as any ?? {}}
310-
/>
311-
<AdvancedOptions
312-
editor={props.editor}
313-
form$={props.form$}
314-
initialLinkType={props.initialLinkType}
315-
linkType={linkType}
316-
model$={model$}
317-
options={editorOptions.linkTypes?.[linkType.id] as any ?? {}}
318-
/>
319-
</>
320-
)}
294+
<Editor
295+
model$={model$}
296+
options={editorOptions.linkTypes?.[linkType.id] as any ?? {}}
297+
/>
298+
<AdvancedOptions
299+
editor={props.editor}
300+
form$={props.form$}
301+
initialLinkType={props.initialLinkType}
302+
linkType={linkType}
303+
model$={model$}
304+
options={editorOptions.linkTypes?.[linkType.id] as any ?? {}}
305+
/>
321306
</ErrorBoundary>
322307

323308
</Layout.Stack>
@@ -402,41 +387,38 @@ const AdvancedOptions: React.FC<{
402387
}> = props => {
403388
const {enabledLinkOptions} = useLatestState(props.editor.state$);
404389

405-
const advancedOptions$ = React.useMemo(() => mapState(props.form$, (form) => {
406-
const activeModel = form.linkModels[form.activeLinkTypeId];
407-
408-
const modelIsDirty = activeModel && props.linkType.isDirty(activeModel);
409-
390+
const formStatus$ = React.useMemo(() => mapState(props.form$, (form) => {
410391
const isOptionSet = Object.values(form.options ?? {}).some(Boolean);
411392

412393
return {
413-
isUsed: isOptionSet || Boolean(activeModel && props.linkType.isAdvanced?.(activeModel)),
414-
enabled: modelIsDirty || (props.initialLinkType && !form.initialLinkWasDeleted ? props.initialLinkType.id === props.linkType.id : false)
394+
isOptionSet,
395+
initialLinkWasDeleted: form.initialLinkWasDeleted
415396
}
416397
}), []);
417398

418-
const advancedOptions = useLatestState(advancedOptions$);
399+
const formStatus = useLatestState(formStatus$);
400+
const model = useLatestState(props.model$);
401+
402+
const modelIsDirty = model && props.linkType.isDirty(model);
403+
404+
const enabled = modelIsDirty || (props.initialLinkType && !formStatus.initialLinkWasDeleted ? props.initialLinkType.id === props.linkType.id : false);
419405

420-
// todo odd state, when removing last set value dialog closes
421-
const [isOpen, setOpen] = React.useState<boolean | undefined>(undefined);
406+
const isUsed = enabled && (formStatus.isOptionSet || Boolean(model && props.linkType.isAdvanced?.(model)));
422407

423-
const toggleOpen = React.useCallback(() => setOpen(openState => {
424-
const prevOpen = openState ?? advancedOptions.isUsed;
425-
return !prevOpen;
426-
}), [advancedOptions]);
408+
const [isOpen, setOpen] = React.useState<boolean>(isUsed);
409+
410+
const toggleOpen = React.useCallback(() => enabled ? setOpen(openState => !openState) : null, [enabled]);
427411

428412
const {AdvancedEditor} = props.linkType;
429413

430414
if (!enabledLinkOptions.length && !AdvancedEditor) {
431415
return null;
432416
}
433417

434-
const isAdvancedOpen = advancedOptions.enabled && (isOpen ?? advancedOptions.isUsed);
435-
436418
return <div className={style.advanced}>
437-
<div className={!advancedOptions.enabled ? style.advancedButtonDisabled : (isAdvancedOpen ? style.advancedButtonIsOpen : style.advancedButton)} onClick={toggleOpen} ><Icon icon='cogs' color={advancedOptions.isUsed ? 'primaryBlue' : undefined} />&nbsp; Advanced &nbsp; <Icon icon={isAdvancedOpen ? 'chevron-up' : 'chevron-down'}/></div>
419+
<div className={!enabled ? style.advancedButtonDisabled : (isOpen ? style.advancedButtonIsOpen : style.advancedButton)} onClick={toggleOpen} ><Icon icon='cogs' color={isUsed ? 'primaryBlue' : undefined} />&nbsp; Advanced &nbsp; <Icon icon={isOpen ? 'chevron-up' : 'chevron-down'}/></div>
438420
{
439-
isAdvancedOpen ? (
421+
isOpen ? (
440422
<div className={style.advancedContents}>
441423
<Layout.Stack>
442424
{AdvancedEditor

packages/neos-ui-link-editor-core/src/application/Dialog/style.module.css

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
cursor: pointer;
1818
line-height: var(--spacing-GoldenUnit);
1919
background: var(--colors-ContrastDarkest);
20+
user-select: none;
2021

2122
border: 1px solid var(--colors-ContrastDark);
2223
position: relative;

packages/neos-ui-link-editor-core/src/application/LinkTypes/Asset/Asset.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import {IconCard, ImageCard} from '../../../presentation';
66
import {MediaBrowser} from './MediaBrowser';
77
import {isSuitableFor} from './AssetSpecification';
88
import {translate} from '@neos-project/neos-ui-i18n';
9-
import {PromiseState, usePromise} from '@neos-project/framework-promise-react';
9+
import {usePromise} from '@neos-project/framework-promise-react';
1010
import backend from '@neos-project/neos-ui-backend-connector';
1111
import {State} from '@neos-project/framework-observable';
1212
import {useLatestState} from '@neos-project/framework-observable-react';
@@ -37,17 +37,17 @@ export const Asset = makeLinkType<AssetLinkModel>('LinkEditor:Asset', ({createEr
3737
return Boolean(model.anchor);
3838
},
3939

40-
useResolvedModel: (link: ILink) => {
40+
convertLinkToModel: (link: ILink) => {
4141
const match = /asset:\/\/([^#]*)(?:#(.*))?/.exec(link.href);
4242

4343
if (!match) {
44-
return PromiseState.forError(createError(`Cannot handle href "${link.href}".`));
44+
throw createError(`Cannot handle href "${link.href}".`);
4545
}
4646

4747
const identifier = match[1];
4848
const anchor = match[2];
4949

50-
return PromiseState.forValue({isDirty: false, identifier, anchor});
50+
return {isDirty: false, identifier, anchor};
5151
},
5252

5353
convertModelToLink: ({identifier, anchor}: AssetLinkModel) => ({

packages/neos-ui-link-editor-core/src/application/LinkTypes/MailTo/MailTo.tsx

Lines changed: 3 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import {ILink, makeLinkType} from '../../../domain';
44
import {IconCard, Layout} from '../../../presentation';
55
import {isSuitableFor} from './MailToSpecification';
66
import {translate} from '@neos-project/neos-ui-i18n';
7-
import {PromiseState} from '@neos-project/framework-promise-react';
87
import {State} from '@neos-project/framework-observable';
98
import {useLatestState} from '@neos-project/framework-observable-react';
109
import {TextArea, TextInput, Tooltip} from '@neos-project/react-ui-components';
@@ -97,15 +96,13 @@ export const MailTo = makeLinkType<MailToLinkModel, MailToOptions>('LinkEditor:M
9796
return true;
9897
},
9998

100-
useResolvedModel: (link: ILink) => {
99+
convertLinkToModel: (link: ILink) => {
101100
if (!link.href.startsWith('mailto:')) {
102-
return PromiseState.forError(
103-
createError(`Cannot handle href "${link.href}".`)
104-
);
101+
throw createError(`Cannot handle href "${link.href}".`);
105102
}
106103
const url = new URL(link.href);
107104

108-
const value = validateEmail({
105+
return validateEmail({
109106
recipient: {
110107
value: url.pathname,
111108
isDirty: false
@@ -127,31 +124,6 @@ export const MailTo = makeLinkType<MailToLinkModel, MailToOptions>('LinkEditor:M
127124
isDirty: false
128125
}
129126
});
130-
131-
// return usePromise(() => new Promise(r => setTimeout(() => r(value), 1000)), [])
132-
133-
return PromiseState.forValue(validateEmail({
134-
recipient: {
135-
value: url.pathname,
136-
isDirty: false
137-
},
138-
subject: {
139-
value: url.searchParams.get('subject') ?? '',
140-
isDirty: false
141-
},
142-
cc: {
143-
value: url.searchParams.get('cc') ?? '',
144-
isDirty: false
145-
},
146-
bcc: {
147-
value: url.searchParams.get('bcc') ?? '',
148-
isDirty: false
149-
},
150-
body: {
151-
value: url.searchParams.get('body') ?? '',
152-
isDirty: false
153-
}
154-
}));
155127
},
156128

157129
convertModelToLink: (email: MailToLinkModel) => {

packages/neos-ui-link-editor-core/src/application/LinkTypes/Node/Node.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* source code.
99
*/
1010
import React from 'react';
11-
import {PromiseState, usePromise} from '@neos-project/framework-promise-react';
11+
import {usePromise} from '@neos-project/framework-promise-react';
1212

1313
import {selectors, useSelector} from '@neos-project/neos-ui-redux-store';
1414
import {Tree} from '@neos-project/neos-ui-link-editor-custom-node-tree';
@@ -103,17 +103,17 @@ export const Node = makeLinkType<NodeLinkModel, NodeLinkOptions>('LinkEditor:Nod
103103
return Boolean(model.anchor);
104104
},
105105

106-
useResolvedModel: (link: ILink) => {
106+
convertLinkToModel: (link: ILink) => {
107107
const match = /node:\/\/([^#]*)(?:#(.*))?/.exec(link.href);
108108

109109
if (!match) {
110-
return PromiseState.forError(createError(`Cannot handle href "${link.href}".`));
110+
throw createError(`Cannot handle href "${link.href}".`);
111111
}
112112

113113
const nodeId = match[1];
114114
const anchor = match[2];
115115

116-
return PromiseState.forValue({isDirty: false, nodeId, anchor});
116+
return {isDirty: false, nodeId, anchor};
117117
},
118118

119119
convertModelToLink: ({nodeId, anchor}: NodeLinkModel) => ({

packages/neos-ui-link-editor-core/src/application/LinkTypes/PhoneNumber/PhoneNumber.tsx

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@ import {translate} from '@neos-project/neos-ui-i18n';
99
import {State} from '@neos-project/framework-observable';
1010
import {useLatestState} from '@neos-project/framework-observable-react';
1111

12-
import {PromiseState} from '@neos-project/framework-promise-react';
13-
1412
type PhoneNumberLinkModel = {
1513
phoneNumber: {
1614
value: string
@@ -48,19 +46,17 @@ export const PhoneNumber = makeLinkType<PhoneNumberLinkModel>('LinkEditor:PhoneN
4846
return trimmedValue !== '' && trimmedValue !== '+';
4947
},
5048

51-
useResolvedModel: (link: ILink) => {
49+
convertLinkToModel: (link: ILink) => {
5250
if (!link.href.startsWith('tel:')) {
53-
return PromiseState.forError(
54-
createError(`Cannot handle href "${link.href}".`)
55-
);
51+
throw createError(`Cannot handle href "${link.href}".`);
5652
}
5753

58-
return PromiseState.forValue(validateModel({
54+
return validateModel({
5955
phoneNumber: {
6056
value: link.href.replace('tel:', ''),
6157
isDirty: false
6258
}
63-
}));
59+
});
6460
},
6561

6662
convertModelToLink: (model: PhoneNumberLinkModel) => {

0 commit comments

Comments
 (0)