Skip to content

Commit 6e3e634

Browse files
y-youngy-young
authored
MBS-11680: Group editing URL relationships by external link (metabrainz#2114)
Implement a new UI for external links editor, where links with the same URL are grouped together, relationship types are shown under the URL as a list, and URLs are numbered for reference in error messages. It addresses the following issues of the current external links editor: * It did not make obvious a URL can be pasted a second time to add another relationship type; * It was not easy to find if a URL has been pasted already (except when using the same relationship type); * It required double editing to change a URL having two relationships; * Error’s position was the same for both relationship type and URL, it wasn’t always obvious which field had to be edited to fix the error. Implementation details: * Additional relationship can be added in two different ways: * Either by pasting the same URL a second time (as before); * Or by clicking the 'Add another relationship' button. * Additional input elements are shown only when needed: * 'Add another relationship' button only if permitted, (auto-selected relationship types currently don’t allow for a second relationship); * 'Remove relationship' button is displayed only if there are several relationships as at least one relationship is required per URL. * Linked URL is highlighted when focused to ease keyboard navigation. * Reference the position of the original URL through a notice when the pasted URL is a duplicate. Users can then press enter or select a different type to confirm merging with the existing one. * Improve layout to prevent relationship type description `?` button to hide other controls and to be unintentionally triggered. * Improve workflow with links that have error by instantly appending new input boxes and not freezing them. * Enable validating URLs in URL editing popover and display URL-related errors only (without re-rendering the whole list). * Fix opening links in a new tab (to prevent editing data loss). * Fix error target of restricted link types (to `RELATIONSHIP`).
1 parent 60ce54d commit 6e3e634

File tree

8 files changed

+1082
-421
lines changed

8 files changed

+1082
-421
lines changed

‎root/static/scripts/edit/URLCleanup.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -4518,7 +4518,7 @@ Object.values(LINK_TYPES).forEach(function (linkType) {
45184518
}
45194519
return {
45204520
result: RESTRICTED_LINK_TYPES.indexOf(id) === -1,
4521-
target: ERROR_TARGETS.URL,
4521+
target: ERROR_TARGETS.RELATIONSHIP,
45224522
};
45234523
};
45244524
}

‎root/static/scripts/edit/components/HelpIcon.js

+8-1
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,18 @@ class HelpIcon extends React.Component {
1818

1919
render() {
2020
return (
21-
<div style={{position: 'relative', display: 'inline-block'}}>
21+
<div
22+
style={{
23+
position: 'relative',
24+
display: 'inline-block',
25+
marginLeft: '10px',
26+
}}
27+
>
2228
<div
2329
className="img icon help"
2430
onMouseEnter={() => this.setState({hover: true})}
2531
onMouseLeave={() => this.setState({hover: false})}
32+
style={{verticalAlign: 'text-top'}}
2633
/>
2734
{this.state.hover &&
2835
<Tooltip

‎root/static/scripts/edit/components/RemoveButton.js

+1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ class RemoveButton extends React.Component {
1313
return (
1414
<button
1515
className="nobutton icon remove-item"
16+
data-index={this.props['data-index']}
1617
onClick={this.props.onClick}
1718
title={this.props.title}
1819
type="button"

‎root/static/scripts/edit/components/URLInputPopover.js

+100-81
Original file line numberDiff line numberDiff line change
@@ -9,112 +9,131 @@
99

1010
import * as React from 'react';
1111
import ButtonPopover from '../../common/components/ButtonPopover';
12-
import type {ErrorTarget} from '../externalLinks';
12+
import type {ErrorT, LinkStateT} from '../externalLinks';
1313
import {ERROR_TARGETS} from '../URLCleanup';
1414

1515
type PropsT = {
16-
errorMessage: string,
17-
errorTarget: ErrorTarget,
18-
onCancel: () => void,
19-
onChange: (SyntheticEvent<HTMLInputElement>) => void,
20-
onToggle: (boolean) => void,
21-
rawUrl: string,
22-
url: string,
16+
cleanupUrl: (string) => string,
17+
link: LinkStateT,
18+
onConfirm: (string) => void,
19+
validateLink: (LinkStateT) => ErrorT | null,
2320
};
2421

2522
const URLInputPopover = (props: PropsT): React.MixedElement => {
2623
const popoverButtonRef = React.useRef(null);
27-
const [isOpen, setIsOpen] = React.useState(false);
24+
const [isOpen, setIsOpen] = React.useState<boolean>(false);
25+
const [link, setLink] = React.useState<LinkStateT>(props.link);
2826

29-
const toggle = (open) => {
30-
/*
31-
* Will be called by ButtonPopover when closed
32-
* either by losing focus or click 'Close' button
33-
*/
27+
React.useEffect(() => {
28+
setLink(props.link);
29+
}, [props.link]);
30+
31+
const toggle = (open: boolean) => {
32+
// Will be called by ButtonPopover when closed by losing focus
33+
if (!open) {
34+
props.onConfirm(link.rawUrl);
35+
}
3436
setIsOpen(open);
35-
props.onToggle(open);
3637
};
3738

38-
const handleCancel = () => {
39-
props.onCancel();
40-
toggle(false);
39+
const handleUrlChange = (event: SyntheticEvent<HTMLInputElement>) => {
40+
const rawUrl = event.currentTarget.value;
41+
setLink({
42+
...link,
43+
rawUrl,
44+
url: props.cleanupUrl(rawUrl),
45+
});
46+
};
47+
48+
const handleConfirm = (closeCallback: () => void) => {
49+
props.onConfirm(link.rawUrl);
50+
closeCallback();
4151
};
4252

4353
const buildPopoverChildren = (
4454
closeAndReturnFocus,
45-
) => (
46-
<form
47-
onSubmit={(event) => {
48-
event.preventDefault();
49-
closeAndReturnFocus();
50-
}}
51-
>
52-
<table>
53-
<tbody>
54-
<tr>
55-
<td className="section">
56-
{addColonText(l('URL'))}
57-
</td>
58-
<td>
59-
<input
60-
className="value raw-url"
61-
onChange={props.onChange}
62-
style={{width: '336px'}}
63-
value={props.rawUrl}
64-
/>
65-
{props.errorMessage &&
66-
props.errorTarget === ERROR_TARGETS.URL &&
67-
<div
68-
className="error field-error target-url"
69-
data-visible="1"
70-
>
71-
{props.errorMessage}
72-
</div>
73-
}
74-
</td>
75-
</tr>
76-
{props.url &&
55+
) => {
56+
const error = props.validateLink(link);
57+
return (
58+
<form
59+
onSubmit={(event) => {
60+
event.preventDefault();
61+
handleConfirm(closeAndReturnFocus);
62+
}}
63+
>
64+
<table>
65+
<tbody>
7766
<tr>
78-
<td className="section" style={{whiteSpace: 'nowrap'}}>
79-
{addColonText(l('Cleaned up to'))}
67+
<td className="section">
68+
{addColonText(l('URL'))}
8069
</td>
8170
<td>
82-
<a
83-
className="clean-url"
84-
href={props.url}
85-
rel="noreferrer"
86-
target="_blank"
87-
>
88-
{props.url}
89-
</a>
71+
<input
72+
className="value raw-url"
73+
onChange={handleUrlChange}
74+
style={{width: '336px'}}
75+
value={link.rawUrl}
76+
/>
77+
{error &&
78+
error.target === ERROR_TARGETS.URL &&
79+
<div
80+
className="error field-error target-url"
81+
data-visible="1"
82+
>
83+
{error.message}
84+
</div>
85+
}
9086
</td>
9187
</tr>
92-
}
93-
</tbody>
94-
</table>
95-
<div className="buttons" style={{display: 'block', marginTop: '1em'}}>
96-
<button
97-
className="negative"
98-
onClick={handleCancel}
99-
type="button"
100-
>
101-
{l('Cancel')}
102-
</button>
103-
<div
104-
className="buttons-right"
105-
style={{float: 'right', textAlign: 'right'}}
106-
>
88+
{link.url &&
89+
<tr>
90+
<td className="section" style={{whiteSpace: 'nowrap'}}>
91+
{addColonText(l('Cleaned up to'))}
92+
</td>
93+
<td>
94+
{error ? link.url : (
95+
<a
96+
className="clean-url"
97+
href={link.url}
98+
rel="noreferrer"
99+
target="_blank"
100+
>
101+
{link.url}
102+
</a>)}
103+
</td>
104+
</tr>
105+
}
106+
</tbody>
107+
</table>
108+
<div className="buttons" style={{display: 'block', marginTop: '1em'}}>
107109
<button
108-
className="positive"
109-
onClick={closeAndReturnFocus}
110+
className="negative"
111+
onClick={() => {
112+
// Reset input field value
113+
setLink(props.link);
114+
// Avoid calling toggle() otherwise changes will be saved
115+
setIsOpen(false);
116+
}}
110117
type="button"
111118
>
112-
{l('Done')}
119+
{l('Cancel')}
113120
</button>
121+
<div
122+
className="buttons-right"
123+
style={{float: 'right', textAlign: 'right'}}
124+
>
125+
<button
126+
className="positive"
127+
onClick={() => handleConfirm(closeAndReturnFocus)}
128+
type="button"
129+
>
130+
{l('Done')}
131+
</button>
132+
</div>
114133
</div>
115-
</div>
116-
</form>
117-
);
134+
</form>
135+
);
136+
};
118137

119138
return (
120139
<ButtonPopover

0 commit comments

Comments
 (0)