Skip to content

Commit b5f00d4

Browse files
acelayaacelaya
authored
Merge pull request #251 from acelaya-forks/feature/refactor-short-url-form
Feature/refactor short url form
2 parents ef69c01 + 81f1aa1 commit b5f00d4

File tree

5 files changed

+106
-119
lines changed

5 files changed

+106
-119
lines changed

‎src/short-urls/CreateShortUrl.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ const CreateShortUrl: FCWithDeps<CreateShortUrlConnectProps, CreateShortUrlDeps>
5454
<ShortUrlForm
5555
initialState={initialState}
5656
saving={shortUrlCreation.saving}
57-
mode={basicMode ? 'create-basic' : 'create'}
57+
basicMode={basicMode}
5858
onSave={async (data) => {
5959
resetCreateShortUrl();
6060
return createShortUrl(data);

‎src/short-urls/EditShortUrl.tsx

-1
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,6 @@ const EditShortUrl: FCWithDeps<EditShortUrlProps, EditShortUrlDeps> = (
7777
<ShortUrlForm
7878
initialState={initialState}
7979
saving={saving}
80-
mode="edit"
8180
onSave={async (shortUrlData) => {
8281
shortUrl && editShortUrl({ ...shortUrl, data: shortUrlData });
8382
}}

‎src/short-urls/ShortUrlForm.tsx

+97-108
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,11 @@
1-
import type { IconProp } from '@fortawesome/fontawesome-svg-core';
21
import { faAndroid, faApple } from '@fortawesome/free-brands-svg-icons';
32
import { faDesktop } from '@fortawesome/free-solid-svg-icons';
43
import { Checkbox, SimpleCard } from '@shlinkio/shlink-frontend-kit';
54
import { clsx } from 'clsx';
65
import { parseISO } from 'date-fns';
7-
import type { ChangeEvent, FC } from 'react';
8-
import { useEffect, useState } from 'react';
9-
import { Button, FormGroup, Input, Row } from 'reactstrap';
10-
import type { InputType } from 'reactstrap/types/lib/Input';
6+
import type { FC, FormEvent } from 'react';
7+
import { useCallback, useMemo, useState } from 'react';
8+
import { Button, Input, Row } from 'reactstrap';
119
import type { ShlinkCreateShortUrlData, ShlinkDeviceLongUrls, ShlinkEditShortUrlData } from '../api-contract';
1210
import type { FCWithDeps } from '../container/utils';
1311
import { componentFactory, useDependencies } from '../container/utils';
@@ -18,18 +16,13 @@ import { IconInput } from '../utils/components/IconInput';
1816
import { formatIsoDate } from '../utils/dates/helpers/date';
1917
import { LabelledDateInput } from '../utils/dates/LabelledDateInput';
2018
import { useFeature } from '../utils/features';
21-
import { handleEventPreventingDefault, hasValue } from '../utils/helpers';
19+
import { hasValue } from '../utils/helpers';
2220
import { ShortUrlFormCheckboxGroup } from './helpers/ShortUrlFormCheckboxGroup';
2321
import { UseExistingIfFoundInfoIcon } from './UseExistingIfFoundInfoIcon';
2422
import './ShortUrlForm.scss';
2523

26-
export type Mode = 'create' | 'create-basic' | 'edit';
27-
28-
type NonDateFields = 'longUrl' | 'customSlug' | 'shortCodeLength' | 'domain' | 'maxVisits' | 'title';
29-
3024
export interface ShortUrlFormProps<T extends ShlinkCreateShortUrlData | ShlinkEditShortUrlData> {
31-
// FIXME Try to get rid of the mode param, and infer creation or edition from initialState if possible
32-
mode: Mode;
25+
basicMode?: boolean;
3326
saving: boolean;
3427
initialState: T;
3528
onSave: (shortUrlData: T) => Promise<unknown>;
@@ -52,98 +45,74 @@ const isCreationData = (data: ShlinkCreateShortUrlData | ShlinkEditShortUrlData)
5245
const isErrorAction = (action: any): boolean => 'error' in action;
5346

5447
const ShortUrlForm: FCWithDeps<ShortUrlFormConnectProps, ShortUrlFormDeps> = (
55-
{ mode, saving, onSave, initialState, tagsList },
48+
{ basicMode = false, saving, onSave, initialState, tagsList },
5649
) => {
5750
const { TagsSelector, DomainSelector } = useDependencies(ShortUrlForm as unknown as ShortUrlFormDeps);
5851
const [shortUrlData, setShortUrlData] = useState(initialState);
59-
const reset = () => setShortUrlData(initialState);
52+
const isCreation = isCreationData(shortUrlData);
6053
const supportsDeviceLongUrls = useFeature('deviceLongUrls');
6154

62-
const isEdit = mode === 'edit';
63-
const isCreation = isCreationData(shortUrlData);
64-
const isBasicMode = mode === 'create-basic';
65-
const changeTags = (tags: string[]) => setShortUrlData((prev) => ({ ...prev, tags }));
66-
const setResettableValue = (value: string, initialValue?: any) => {
55+
const reset = useCallback(() => setShortUrlData(initialState), [initialState]);
56+
const setResettableValue = useCallback((value: string, initialValue?: any) => {
6757
if (hasValue(value)) {
6858
return value;
6959
}
7060

7161
// If an initial value was provided for this when the input is "emptied", explicitly set it to null so that the
7262
// value gets removed. Otherwise, set undefined so that it gets ignored.
7363
return hasValue(initialValue) ? null : undefined;
74-
};
75-
const submit = handleEventPreventingDefault(async () => onSave(shortUrlData)
76-
.then((result: any) => !isEdit && !isErrorAction(result) && reset())
77-
.catch(() => {}));
64+
}, []);
65+
const changeDeviceLongUrl = useCallback(
66+
(id: keyof ShlinkDeviceLongUrls, url: string) => setShortUrlData(({ deviceLongUrls = {}, ...prev }) => ({
67+
...prev,
68+
deviceLongUrls: {
69+
...deviceLongUrls,
70+
[id]: setResettableValue(url, initialState.deviceLongUrls?.[id]),
71+
},
72+
})),
73+
[initialState.deviceLongUrls, setResettableValue],
74+
);
75+
const changeTags = useCallback((tags: string[]) => setShortUrlData((prev) => ({ ...prev, tags })), []);
7876

79-
useEffect(() => {
80-
setShortUrlData(initialState);
81-
}, [initialState]);
77+
const submit = useCallback(async (e: FormEvent) => {
78+
e.preventDefault();
79+
return onSave(shortUrlData)
80+
.then((result: any) => isCreation && !isErrorAction(result) && reset())
81+
.catch(() => {});
82+
}, [isCreation, onSave, reset, shortUrlData]);
8283

83-
// TODO Consider extracting these functions to local components
84-
const renderOptionalInput = (
85-
id: NonDateFields,
86-
placeholder: string,
87-
type: InputType = 'text',
88-
props: any = {},
89-
fromGroupProps = {},
90-
) => (
91-
<FormGroup {...fromGroupProps}>
84+
const basicComponents = useMemo(() => (
85+
<div className="d-flex flex-column gap-3">
9286
<Input
93-
name={id}
94-
id={id}
95-
type={type}
96-
placeholder={placeholder}
97-
// @ts-expect-error FIXME Make sure id is a key from T
98-
value={shortUrlData[id] ?? ''}
99-
onChange={props.onChange ?? ((e) => setShortUrlData((prev) => ({ ...prev, [id]: e.target.value })))}
100-
{...props}
87+
bsSize="lg"
88+
type="url"
89+
placeholder="URL to be shortened"
90+
required
91+
value={shortUrlData.longUrl}
92+
onChange={(e) => setShortUrlData((prev) => ({ ...prev, longUrl: e.target.value }))}
10193
/>
102-
</FormGroup>
103-
);
104-
const renderDeviceLongUrlInput = (id: keyof ShlinkDeviceLongUrls, placeholder: string, icon: IconProp) => (
105-
<IconInput
106-
icon={icon}
107-
name={id}
108-
id={id}
109-
type="url"
110-
placeholder={placeholder}
111-
value={shortUrlData.deviceLongUrls?.[id] ?? ''}
112-
onChange={(e) => setShortUrlData((prev) => ({
113-
...prev,
114-
deviceLongUrls: {
115-
...(prev.deviceLongUrls ?? {}),
116-
[id]: setResettableValue(e.target.value, initialState.deviceLongUrls?.[id]),
117-
},
118-
}))}
119-
/>
120-
);
121-
const basicComponents = (
122-
<>
123-
<FormGroup>
124-
<Input
125-
name="longUrl"
126-
bsSize="lg"
127-
type="url"
128-
placeholder="URL to be shortened"
129-
required
130-
value={shortUrlData.longUrl}
131-
onChange={(e) => setShortUrlData((prev) => ({ ...prev, longUrl: e.target.value }))}
132-
/>
133-
</FormGroup>
13494
<Row>
135-
{isBasicMode && renderOptionalInput('customSlug', 'Custom slug', 'text', { bsSize: 'lg' }, { className: 'col-lg-6' })}
136-
<div className={isBasicMode ? 'col-lg-6 mb-3' : 'col-12'}>
95+
{basicMode && isCreation && (
96+
<div className="col-lg-6 mb-3">
97+
<Input
98+
bsSize="lg"
99+
placeholder="Custom slug"
100+
value={shortUrlData.customSlug ?? ''}
101+
onChange={(e) => setShortUrlData((prev) => ({ ...prev, customSlug: e.target.value }))}
102+
/>
103+
</div>
104+
)}
105+
<div className={basicMode ? 'col-lg-6 mb-3' : 'col-12'}>
137106
<TagsSelector tags={tagsList.tags} selectedTags={shortUrlData.tags ?? []} onChange={changeTags} />
138107
</div>
139108
</Row>
140-
</>
141-
);
109+
</div>
110+
), [TagsSelector, basicMode, changeTags, isCreation, shortUrlData, tagsList.tags]);
142111

143112
return (
144113
<form name="shortUrlForm" className="short-url-form" onSubmit={submit}>
145-
{isBasicMode && basicComponents}
146-
{!isBasicMode && (
114+
{basicMode && basicComponents}
115+
{!basicMode && (
147116
<>
148117
<Row>
149118
<div className={clsx('mb-3', { 'col-sm-6': supportsDeviceLongUrls, 'col-12': !supportsDeviceLongUrls })}>
@@ -153,46 +122,69 @@ const ShortUrlForm: FCWithDeps<ShortUrlFormConnectProps, ShortUrlFormDeps> = (
153122
</div>
154123
{supportsDeviceLongUrls && (
155124
<div className="col-sm-6 mb-3">
156-
<SimpleCard title="Device-specific long URLs">
157-
<FormGroup>
158-
{renderDeviceLongUrlInput('android', 'Android-specific redirection', faAndroid)}
159-
</FormGroup>
160-
<FormGroup>
161-
{renderDeviceLongUrlInput('ios', 'iOS-specific redirection', faApple)}
162-
</FormGroup>
163-
{renderDeviceLongUrlInput('desktop', 'Desktop-specific redirection', faDesktop)}
125+
<SimpleCard title="Device-specific long URLs" bodyClassName="d-flex flex-column gap-3">
126+
<IconInput
127+
type="url"
128+
icon={faAndroid}
129+
placeholder="Android-specific redirection"
130+
value={shortUrlData.deviceLongUrls?.android ?? ''}
131+
onChange={({ target }) => changeDeviceLongUrl('android', target.value)}
132+
/>
133+
<IconInput
134+
type="url"
135+
icon={faApple}
136+
placeholder="iOS-specific redirection"
137+
value={shortUrlData.deviceLongUrls?.ios ?? ''}
138+
onChange={({ target }) => changeDeviceLongUrl('ios', target.value)}
139+
/>
140+
<IconInput
141+
type="url"
142+
icon={faDesktop}
143+
placeholder="Desktop-specific redirection"
144+
value={shortUrlData.deviceLongUrls?.desktop ?? ''}
145+
onChange={({ target }) => changeDeviceLongUrl('desktop', target.value)}
146+
/>
164147
</SimpleCard>
165148
</div>
166149
)}
167150
</Row>
168151

169152
<Row>
170153
<div className="col-sm-6 mb-3">
171-
<SimpleCard title="Customize the short URL">
172-
{renderOptionalInput('title', 'Title', 'text', {
173-
onChange: ({ target }: ChangeEvent<HTMLInputElement>) => setShortUrlData((prev) => ({
154+
<SimpleCard title="Customize the short URL" bodyClassName="d-flex flex-column gap-3">
155+
<Input
156+
placeholder="Title"
157+
value={shortUrlData.title ?? ''}
158+
onChange={({ target }) => setShortUrlData((prev) => ({
174159
...prev,
175160
title: setResettableValue(target.value, initialState.title),
176-
})),
177-
})}
178-
{!isEdit && isCreation && (
161+
}))}
162+
/>
163+
{isCreation && (
179164
<>
180165
<Row>
181-
<div className="col-lg-6">
182-
{renderOptionalInput('customSlug', 'Custom slug', 'text', {
183-
disabled: hasValue(shortUrlData.shortCodeLength),
184-
})}
166+
<div className="col-lg-6 mb-3 mb-lg-0">
167+
<Input
168+
placeholder="Custom slug"
169+
value={shortUrlData.customSlug ?? ''}
170+
onChange={(e) => setShortUrlData((prev) => ({ ...prev, customSlug: e.target.value }))}
171+
disabled={hasValue(shortUrlData.shortCodeLength)}
172+
/>
185173
</div>
186174
<div className="col-lg-6">
187-
{renderOptionalInput('shortCodeLength', 'Short code length', 'number', {
188-
min: 4,
189-
disabled: hasValue(shortUrlData.customSlug),
190-
})}
175+
<Input
176+
type="number"
177+
placeholder="Short code length"
178+
value={shortUrlData.shortCodeLength ?? ''}
179+
onChange={(e) => setShortUrlData((prev) => ({ ...prev, shortCodeLength: e.target.value }))}
180+
min={4}
181+
disabled={hasValue(shortUrlData.customSlug)}
182+
/>
191183
</div>
192184
</Row>
193185
<DomainSelector
194186
value={shortUrlData.domain}
195-
onChange={(domain?: string) => setShortUrlData((prev) => ({ ...prev, domain }))}
187+
onChange={(domain) => setShortUrlData((prev) => ({ ...prev, domain }))}
196188
/>
197189
</>
198190
)}
@@ -207,7 +199,6 @@ const ShortUrlForm: FCWithDeps<ShortUrlFormConnectProps, ShortUrlFormDeps> = (
207199
label="Enabled since"
208200
withTime
209201
maxDate={shortUrlData.validUntil ? toDate(shortUrlData.validUntil) : undefined}
210-
name="validSince"
211202
value={shortUrlData.validSince ? toDate(shortUrlData.validSince) : null}
212203
onChange={(date) => setShortUrlData((prev) => ({ ...prev, validSince: formatIsoDate(date) }))}
213204
/>
@@ -217,7 +208,6 @@ const ShortUrlForm: FCWithDeps<ShortUrlFormConnectProps, ShortUrlFormDeps> = (
217208
label="Enabled until"
218209
withTime
219210
minDate={shortUrlData.validSince ? toDate(shortUrlData.validSince) : undefined}
220-
name="validUntil"
221211
value={shortUrlData.validUntil ? toDate(shortUrlData.validUntil) : null}
222212
onChange={(date) => setShortUrlData((prev) => ({ ...prev, validUntil: formatIsoDate(date) }))}
223213
/>
@@ -227,7 +217,6 @@ const ShortUrlForm: FCWithDeps<ShortUrlFormConnectProps, ShortUrlFormDeps> = (
227217
<div>
228218
<label htmlFor="maxVisits" className="mb-1">Maximum visits allowed:</label>
229219
<Input
230-
name="maxVisits"
231220
id="maxVisits"
232221
type="number"
233222
min={1}
@@ -253,7 +242,7 @@ const ShortUrlForm: FCWithDeps<ShortUrlFormConnectProps, ShortUrlFormDeps> = (
253242
>
254243
Validate URL
255244
</ShortUrlFormCheckboxGroup>
256-
{!isEdit && isCreation && (
245+
{isCreation && (
257246
<p>
258247
<Checkbox
259248
inline

‎src/utils/components/IconInput.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import type { InputProps } from 'reactstrap';
77
import { Input } from 'reactstrap';
88
import './IconInput.scss';
99

10-
type IconInputProps = InputProps & {
10+
export type IconInputProps = InputProps & {
1111
icon: IconProp;
1212
};
1313

‎test/short-urls/ShortUrlForm.test.tsx

+7-8
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,14 @@ import { screen } from '@testing-library/react';
22
import type { UserEvent } from '@testing-library/user-event';
33
import { fromPartial } from '@total-typescript/shoehorn';
44
import { formatISO } from 'date-fns';
5-
import type { Mode } from '../../src/short-urls/ShortUrlForm';
65
import { ShortUrlFormFactory } from '../../src/short-urls/ShortUrlForm';
76
import { FeaturesProvider } from '../../src/utils/features';
87
import { checkAccessibility } from '../__helpers__/accessibility';
98
import { renderWithEvents } from '../__helpers__/setUpTest';
109

1110
type SetUpOptions = {
1211
withDeviceLongUrls ?: boolean;
13-
mode?: Mode;
12+
basicMode?: boolean;
1413
title?: string | null;
1514
};
1615

@@ -20,11 +19,11 @@ describe('<ShortUrlForm />', () => {
2019
TagsSelector: () => <span>TagsSelector</span>,
2120
DomainSelector: () => <span>DomainSelector</span>,
2221
}));
23-
const setUp = ({ withDeviceLongUrls = false, mode = 'create', title }: SetUpOptions = {}) =>
22+
const setUp = ({ withDeviceLongUrls = false, basicMode, title }: SetUpOptions = {}) =>
2423
renderWithEvents(
2524
<FeaturesProvider value={fromPartial({ deviceLongUrls: withDeviceLongUrls })}>
2625
<ShortUrlForm
27-
mode={mode}
26+
basicMode={basicMode}
2827
saving={false}
2928
initialState={{
3029
validateUrl: true,
@@ -104,12 +103,12 @@ describe('<ShortUrlForm />', () => {
104103
);
105104

106105
it.each([
107-
['create' as Mode, 5],
108-
['create-basic' as Mode, 0],
106+
[false, 5],
107+
[true, 0],
109108
])(
110109
'renders expected amount of cards based on server capabilities and mode',
111-
(mode, expectedAmountOfCards) => {
112-
setUp({ mode });
110+
(basicMode, expectedAmountOfCards) => {
111+
setUp({ basicMode });
113112
const cards = screen.queryAllByRole('heading');
114113

115114
expect(cards).toHaveLength(expectedAmountOfCards);

0 commit comments

Comments
 (0)