Skip to content

Commit aeeb835

Browse files
committed
SmartUrlInput: Remove not-so-helpful "smart"ness
And just use an ordinary controlled TextInput, always stretched to full width, and never just 1px. The major reason for this is to fix #5228 (can't paste org URL): native copy-paste is activated by long-pressing in the input element itself. But long-pressing is really hard when the element is only 1px wide! Include some placeholder text to help people have some idea of what to put here. Don't require or prefill with "https://"; instead, fill that in if no supported scheme is provided. ("Supported scheme" just means "http://" or "https://"; other schemes just don't make sense in this context.) See discussion at https://chat.zulip.org/#narrow/stream/48-mobile/topic/can't.20paste.20org.20URL/near/1327170 Fixes: #5228
1 parent 393dc26 commit aeeb835

File tree

4 files changed

+20
-272
lines changed

4 files changed

+20
-272
lines changed

src/common/SmartUrlInput.js

Lines changed: 10 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,31 @@
11
/* @flow strict-local */
22
import React, { useState, useRef, useCallback, useContext } from 'react';
33
import type { Node } from 'react';
4-
import { Platform, TextInput, TouchableWithoutFeedback, View, Keyboard } from 'react-native';
4+
import { Platform, TextInput, View, Keyboard } from 'react-native';
55
import { useFocusEffect } from '@react-navigation/native';
66
import type { ViewStyleProp } from 'react-native/Libraries/StyleSheet/StyleSheet';
77

88
import type { AppNavigationProp } from '../nav/AppNavigator';
9-
import { ThemeContext, createStyleSheet } from '../styles';
10-
import { autocompleteRealmPieces, autocompleteRealm, fixRealmUrl } from '../utils/url';
11-
import ZulipText from './ZulipText';
9+
import { ThemeContext, createStyleSheet, HALF_COLOR } from '../styles';
1210

1311
const styles = createStyleSheet({
1412
wrapper: {
1513
flexDirection: 'row',
1614
opacity: 0.8,
1715
},
1816
realmInput: {
17+
flex: 1,
1918
padding: 0,
2019
fontSize: 20,
2120
},
22-
realmPlaceholder: {
23-
opacity: 0.75,
24-
},
25-
realmInputEmpty: {
26-
width: 1,
27-
},
2821
});
2922

3023
type Props = $ReadOnly<{|
3124
// TODO: Currently this type is acceptable because the only
3225
// `navigation` prop we pass to a `SmartUrlInput` instance is the
3326
// one from a component on AppNavigator.
3427
navigation: AppNavigationProp<>,
28+
3529
style?: ViewStyleProp,
3630
onChangeText: (value: string) => void,
3731
onSubmitEditing: () => Promise<void>,
@@ -94,20 +88,12 @@ function useRn19366Workaround(textInputRef) {
9488
export default function SmartUrlInput(props: Props): Node {
9589
const { style, onChangeText, onSubmitEditing, enablesReturnKeyAutomatically } = props;
9690

97-
const defaultProtocol = 'https://';
98-
const defaultOrganization = 'your-org';
99-
const defaultDomain = 'zulipchat.com';
100-
10191
// We should replace the fixme with
10292
// `React$ElementRef<typeof TextInput>` when we can. Currently, that
10393
// would make `.current` be `any(implicit)`, which we don't want;
10494
// this is probably down to bugs in Flow's special support for React.
10595
const textInputRef = useRef<$FlowFixMe>();
10696

107-
/**
108-
* The actual input string, exactly as entered by the user,
109-
* without modifications by autocomplete.
110-
*/
11197
const [value, setValue] = useState<string>('');
11298

11399
const themeContext = useContext(ThemeContext);
@@ -135,53 +121,20 @@ export default function SmartUrlInput(props: Props): Node {
135121
const handleChange = useCallback(
136122
(_value: string) => {
137123
setValue(_value);
138-
139-
onChangeText(
140-
fixRealmUrl(
141-
autocompleteRealm(_value, { protocol: defaultProtocol, domain: defaultDomain }),
142-
),
143-
);
124+
onChangeText(_value);
144125
},
145-
[defaultDomain, defaultProtocol, onChangeText],
126+
[onChangeText],
146127
);
147128

148-
// When the "placeholder parts" are pressed, i.e., the parts of the URL
149-
// line that aren't the TextInput itself, we still want to focus the
150-
// TextInput.
151-
// TODO(?): Is it a confusing UX to have a line that looks and acts like
152-
// a text input, but parts of it aren't really?
153-
const urlPress = useCallback(() => {
154-
if (textInputRef.current) {
155-
// `.current` is not type-checked; see definition.
156-
textInputRef.current.focus();
157-
}
158-
}, []);
159-
160129
useRn19366Workaround(textInputRef);
161130

162-
const renderPlaceholderPart = (text: string) => (
163-
<TouchableWithoutFeedback onPress={urlPress}>
164-
<ZulipText
165-
style={[styles.realmInput, { color: themeContext.color }, styles.realmPlaceholder]}
166-
text={text}
167-
/>
168-
</TouchableWithoutFeedback>
169-
);
170-
171-
const [prefix, , suffix] = autocompleteRealmPieces(value, {
172-
domain: defaultDomain,
173-
protocol: defaultProtocol,
174-
});
175-
176131
return (
177132
<View style={[styles.wrapper, style]}>
178-
{prefix !== null && renderPlaceholderPart(prefix)}
179133
<TextInput
180-
style={[
181-
styles.realmInput,
182-
{ color: themeContext.color },
183-
value.length === 0 && styles.realmInputEmpty,
184-
]}
134+
value={value}
135+
placeholder="your-org.zulipchat.com"
136+
placeholderTextColor={HALF_COLOR}
137+
style={[styles.realmInput, { color: themeContext.color }]}
185138
autoFocus
186139
autoCorrect={false}
187140
autoCapitalize="none"
@@ -194,8 +147,6 @@ export default function SmartUrlInput(props: Props): Node {
194147
enablesReturnKeyAutomatically={enablesReturnKeyAutomatically}
195148
ref={textInputRef}
196149
/>
197-
{!value && renderPlaceholderPart(defaultOrganization)}
198-
{suffix !== null && renderPlaceholderPart(suffix)}
199150
</View>
200151
);
201152
}

src/start/RealmInputScreen.js

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,14 @@ type State = {|
2727
progress: boolean,
2828
|};
2929

30+
const urlFromInputValue = (realmInputValue: string): URL | void => {
31+
const withScheme = /^https?:\/\//.test(realmInputValue)
32+
? realmInputValue
33+
: `https://${realmInputValue}`;
34+
35+
return tryParseUrl(withScheme);
36+
};
37+
3038
export default class RealmInputScreen extends PureComponent<Props, State> {
3139
state: State = {
3240
progress: false,
@@ -37,7 +45,7 @@ export default class RealmInputScreen extends PureComponent<Props, State> {
3745
tryRealm: () => Promise<void> = async () => {
3846
const { realmInputValue } = this.state;
3947

40-
const parsedRealm = tryParseUrl(realmInputValue);
48+
const parsedRealm = urlFromInputValue(realmInputValue);
4149
if (!parsedRealm) {
4250
this.setState({ error: 'Please enter a valid URL' });
4351
return;
@@ -106,7 +114,7 @@ export default class RealmInputScreen extends PureComponent<Props, State> {
106114
text="Enter"
107115
progress={progress}
108116
onPress={this.tryRealm}
109-
disabled={tryParseUrl(realmInputValue) === undefined}
117+
disabled={urlFromInputValue(realmInputValue) === undefined}
110118
/>
111119
</Screen>
112120
);

src/utils/__tests__/url-test.js

Lines changed: 0 additions & 156 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,11 @@ import {
44
getResource,
55
isUrlOnRealm,
66
parseProtocol,
7-
fixRealmUrl,
8-
autocompleteRealmPieces,
9-
autocompleteRealm,
107
isUrlAbsolute,
118
isUrlRelative,
129
isUrlPathAbsolute,
1310
} from '../url';
1411
import type { Auth } from '../../types';
15-
import type { AutocompletionDefaults } from '../url';
1612

1713
const urlClassifierCases = {
1814
// These data are mostly a selection from this resource:
@@ -161,155 +157,3 @@ describe('parseProtocol', () => {
161157
expect(parseProtocol('\xA0http://example.org')).toEqual(['http://', 'example.org']);
162158
});
163159
});
164-
165-
describe('fixRealmUrl', () => {
166-
test('undefined input results in empty string', () => {
167-
expect(fixRealmUrl()).toEqual('');
168-
});
169-
170-
test('empty url input results in empty string', () => {
171-
expect(fixRealmUrl('')).toEqual('');
172-
});
173-
174-
test('when a realm url is missing a protocol, prepend https', () => {
175-
expect(fixRealmUrl('example.com')).toEqual('https://example.com');
176-
});
177-
178-
test('when a realm url has a trailing "/" remove it', () => {
179-
expect(fixRealmUrl('https://example.com/')).toEqual('https://example.com');
180-
});
181-
182-
test('when a realm url has two trailing "/" remove them', () => {
183-
expect(fixRealmUrl('https://example.com//')).toEqual('https://example.com');
184-
});
185-
186-
test('when input url is correct, do not change it', () => {
187-
expect(fixRealmUrl('https://example.com')).toEqual('https://example.com');
188-
});
189-
190-
test('remove white-space around input', () => {
191-
expect(fixRealmUrl(' https://example.com/ ')).toEqual('https://example.com');
192-
});
193-
194-
test('remove white-space inside input', () => {
195-
const result = fixRealmUrl('https://subdomain .example. com/ ');
196-
expect(result).toEqual('https://subdomain.example.com');
197-
});
198-
});
199-
200-
describe('autocompleteRealmPieces', () => {
201-
const exampleData: AutocompletionDefaults = {
202-
protocol: 'http://',
203-
domain: 'example.com',
204-
};
205-
206-
test('the empty string yields reasonable values', () => {
207-
const [head, , tail] = autocompleteRealmPieces('', exampleData);
208-
expect(head).toEqual('http://');
209-
expect(tail).toEqual('.example.com');
210-
});
211-
212-
/* Test that input value is unchanged.
213-
214-
Future versions of `autocompleteRealmPieces` may alter certain inputs --
215-
for example, by trimming spaces, standardizing to lowercase, or escaping
216-
via punycode -- but the particular values tested here should all remain
217-
unaltered.
218-
*/
219-
const doSimpleCompletion = (input: string, data?: AutocompletionDefaults) => {
220-
const [head, output, tail] = autocompleteRealmPieces(input, data ?? exampleData);
221-
expect(input).toEqual(output);
222-
return [head, tail];
223-
};
224-
225-
test('a plain word is fully autocompleted', () => {
226-
const [head, tail] = doSimpleCompletion('host-name');
227-
expect(head).toEqual('http://');
228-
expect(tail).toEqual('.example.com');
229-
});
230-
231-
test('an explicit `http` is recognized', () => {
232-
const [head, tail] = doSimpleCompletion('http://host-name');
233-
expect(head).toBeFalsy();
234-
expect(tail).toEqual('.example.com');
235-
});
236-
237-
test('an explicit `https` is recognized', () => {
238-
const [head, tail] = doSimpleCompletion('https://host-name');
239-
expect(head).toBeFalsy();
240-
expect(tail).toEqual('.example.com');
241-
});
242-
243-
test('an explicit IPv4 is recognized', () => {
244-
const [head, tail] = doSimpleCompletion('23.6.64.128');
245-
expect(head).toBeTruthy();
246-
expect(tail).toBeFalsy();
247-
});
248-
249-
test('an explicit IPv6 is recognized', () => {
250-
const [head, tail] = doSimpleCompletion('[2a02:26f0:12f:293:0:0:0:255e]');
251-
expect(head).toBeTruthy();
252-
expect(tail).toBeFalsy();
253-
});
254-
255-
test('localhost with an explicit port is recognized', () => {
256-
const [head, tail] = doSimpleCompletion('localhost:9991');
257-
expect(head).toBeTruthy();
258-
expect(tail).toBeFalsy();
259-
});
260-
261-
test('full host name is recognized', () => {
262-
const [head, tail] = doSimpleCompletion('my-server.example.com');
263-
expect(head).toBeTruthy();
264-
expect(tail).toBeFalsy();
265-
});
266-
267-
test('full host and protocol are recognized', () => {
268-
const [head, tail] = doSimpleCompletion('http://my-server.com');
269-
expect(head).toBeFalsy();
270-
expect(tail).toBeFalsy();
271-
});
272-
273-
test('fully explicit localhost is recognized', () => {
274-
const [head, tail] = doSimpleCompletion('http://localhost:9991');
275-
expect(head).toBeFalsy();
276-
expect(tail).toBeFalsy();
277-
});
278-
});
279-
280-
describe('autocompleteRealm', () => {
281-
const zulipData: AutocompletionDefaults = {
282-
protocol: 'https://',
283-
domain: 'zulipchat.com',
284-
};
285-
286-
test('when no value is entered return empty string', () => {
287-
const result = autocompleteRealm('', zulipData);
288-
expect(result).toEqual('');
289-
});
290-
291-
test('when a protocol is provided, use it', () => {
292-
const result = autocompleteRealm('http://example', zulipData);
293-
expect(result).toEqual('http://example.zulipchat.com');
294-
});
295-
296-
test('do not use any other protocol than http and https', () => {
297-
const result = autocompleteRealm('ftp://example', zulipData);
298-
expect(result).toStartWith('https://ftp://');
299-
});
300-
301-
test('if the hostname contains a dot, consider it complete', () => {
302-
const result = autocompleteRealm('mydomain.org', zulipData);
303-
expect(result).toEqual('https://mydomain.org');
304-
});
305-
306-
test('if the hostname contains multiple dots, consider it complete', () => {
307-
const result = autocompleteRealm('subdomain.mydomain.org', zulipData);
308-
expect(result).toEqual('https://subdomain.mydomain.org');
309-
});
310-
311-
test('if the hostname contains a colon, consider it complete', () => {
312-
const result = autocompleteRealm('localhost:9991', zulipData);
313-
expect(result).toEqual('https://localhost:9991');
314-
});
315-
});

0 commit comments

Comments
 (0)