realm input screen: Offer nicer experience for copied URL

src/chat/ChatScreen.js

@@ -29,7 +29,7 @@ import { getAuth, getCaughtUpForNarrow } from '../selectors';
 import { showErrorAlert } from '../utils/info';
 import { TranslationContext } from '../boot/TranslationProvider';
 import * as api from '../api';
-import { useEdgeTriggeredEffect } from '../reactUtils';
+import { useConditionalEffect } from '../reactUtils';
 
 type Props = $ReadOnly<{|
   navigation: AppNavigationProp<'chat'>,
@@ -71,9 +71,9 @@ const useMessagesWithFetch = args => {
   // like using instance variables in class components:
   //   https://reactjs.org/docs/hooks-faq.html#is-there-something-like-instance-variables
   const shouldFetchWhenNextFocused = React.useRef<boolean>(false);
-  const scheduleFetch = () => {
+  const scheduleFetch = useCallback(() => {
     shouldFetchWhenNextFocused.current = true;
-  };
+  }, []);
 
   const [fetchError, setFetchError] = React.useState<mixed>(null);
 
@@ -92,7 +92,7 @@ const useMessagesWithFetch = args => {
     if (eventQueueId !== null) {
       scheduleFetch();
     }
-  }, [eventQueueId]);
+  }, [eventQueueId, scheduleFetch]);
 
   // If we stop having any data at all about the messages in this narrow --
   // we don't know any, and nor do we know if there are some -- then
@@ -104,7 +104,7 @@ const useMessagesWithFetch = args => {
   //   isFetching false, even though the fetch effect will cause a rerender
   //   with isFetching true.  It'd be nice to avoid that.
   const nothingKnown = messages.length === 0 && !caughtUp.older && !caughtUp.newer;
-  useEdgeTriggeredEffect(scheduleFetch, nothingKnown, true);
+  useConditionalEffect(scheduleFetch, nothingKnown);
 
   // On first mount, fetch.  (This also makes a fetch no longer scheduled,
   // so the if-scheduled fetch below doesn't also fire.)

src/common/SmartUrlInput.js

@@ -1,81 +1,46 @@
 /* @flow strict-local */
 import React, { useState, useRef, useCallback, useContext } from 'react';
 import type { Node } from 'react';
-import { TextInput, TouchableWithoutFeedback, View } from 'react-native';
+import { TextInput, View } from 'react-native';
 import { useFocusEffect } from '@react-navigation/native';
 import type { ViewStyleProp } from 'react-native/Libraries/StyleSheet/StyleSheet';
 
 import type { AppNavigationProp } from '../nav/AppNavigator';
-import { ThemeContext, createStyleSheet } from '../styles';
-import { autocompleteRealmPieces, autocompleteRealm, fixRealmUrl } from '../utils/url';
-import type { Protocol } from '../utils/url';
-import ZulipText from './ZulipText';
+import { ThemeContext, createStyleSheet, HALF_COLOR } from '../styles';
 
 const styles = createStyleSheet({
   wrapper: {
     flexDirection: 'row',
     opacity: 0.8,
   },
   realmInput: {
+    flex: 1,
     padding: 0,
     fontSize: 20,
   },
-  realmPlaceholder: {
-    opacity: 0.75,
-  },
-  realmInputEmpty: {
-    width: 1,
-  },
 });
 
 type Props = $ReadOnly<{|
-  /**
-   * The protocol which will be used if the user doesn't specify one.
-   * Should almost certainly be "https://".
-   */
-  defaultProtocol: Protocol,
-  /**
-   * The example organization name that will be displayed while the
-   * entry field is empty. Appears, briefly, as the initial (lowest-
-   * level) component of the realm's domain.
-   */
-  defaultOrganization: string,
-  /**
-   * The default domain to which the user's input will be appended, if
-   * it appears not to contain an explicit domain.
-   */
-  defaultDomain: string,
   // TODO: Currently this type is acceptable because the only
   // `navigation` prop we pass to a `SmartUrlInput` instance is the
   // one from a component on AppNavigator.
   navigation: AppNavigationProp<>,
+
   style?: ViewStyleProp,
   onChangeText: (value: string) => void,
   onSubmitEditing: () => Promise<void>,
   enablesReturnKeyAutomatically: boolean,
 |}>;
 
 export default function SmartUrlInput(props: Props): Node {
-  const {
-    defaultProtocol,
-    defaultOrganization,
-    defaultDomain,
-    style,
-    onChangeText,
-    onSubmitEditing,
-    enablesReturnKeyAutomatically,
-  } = props;
+  const { style, onChangeText, onSubmitEditing, enablesReturnKeyAutomatically } = props;
 
   // We should replace the fixme with
   // `React$ElementRef<typeof TextInput>` when we can. Currently, that
   // would make `.current` be `any(implicit)`, which we don't want;
   // this is probably down to bugs in Flow's special support for React.
   const textInputRef = useRef<$FlowFixMe>();
 
-  /**
-   * The actual input string, exactly as entered by the user,
-   * without modifications by autocomplete.
-   */
   const [value, setValue] = useState<string>('');
 
   const themeContext = useContext(ThemeContext);
@@ -86,6 +51,14 @@ export default function SmartUrlInput(props: Props): Node {
   useFocusEffect(
     useCallback(() => {
       if (textInputRef.current) {
+        // Sometimes the effect of this `.focus()` is immediately undone
+        // (the keyboard is closed) by a Keyboard.dismiss() from React
+        // Navigation's internals. Seems like a complex bug, but the symptom
+        // isn't terrible, it just means that on back-navigating to this
+        // screen, sometimes the keyboard flicks open then closed, instead
+        // of just opening. Shrug. See
+        //   https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/realm-input/near/1346690
+        //
         // `.current` is not type-checked; see definition.
         textInputRef.current.focus();
       }
@@ -95,52 +68,18 @@ export default function SmartUrlInput(props: Props): Node {
   const handleChange = useCallback(
     (_value: string) => {
       setValue(_value);
-
-      onChangeText(
-        fixRealmUrl(
-          autocompleteRealm(_value, { protocol: defaultProtocol, domain: defaultDomain }),
-        ),
-      );
+      onChangeText(_value);
     },
-    [defaultDomain, defaultProtocol, onChangeText],
+    [onChangeText],
   );
 
-  const urlPress = useCallback(() => {
-    if (textInputRef.current) {
-      // `.current` is not type-checked; see definition.
-      textInputRef.current.blur();
-      setTimeout(() => {
-        if (textInputRef.current) {
-          // `.current` is not type-checked; see definition.
-          textInputRef.current.focus();
-        }
-      }, 100);
-    }
-  }, []);
-
-  const renderPlaceholderPart = (text: string) => (
-    <TouchableWithoutFeedback onPress={urlPress}>
-      <ZulipText
-        style={[styles.realmInput, { color: themeContext.color }, styles.realmPlaceholder]}
-        text={text}
-      />
-    </TouchableWithoutFeedback>
-  );
-
-  const [prefix, , suffix] = autocompleteRealmPieces(value, {
-    domain: defaultDomain,
-    protocol: defaultProtocol,
-  });
-
   return (
     <View style={[styles.wrapper, style]}>
-      {prefix !== null && renderPlaceholderPart(prefix)}
       <TextInput
-        style={[
-          styles.realmInput,
-          { color: themeContext.color },
-          value.length === 0 && styles.realmInputEmpty,
-        ]}
+        value={value}
+        placeholder="your-org.zulipchat.com"
+        placeholderTextColor={HALF_COLOR}
+        style={[styles.realmInput, { color: themeContext.color }]}
         autoFocus
         autoCorrect={false}
         autoCapitalize="none"
@@ -153,8 +92,6 @@ export default function SmartUrlInput(props: Props): Node {
         enablesReturnKeyAutomatically={enablesReturnKeyAutomatically}
         ref={textInputRef}
       />
-      {!value && renderPlaceholderPart(defaultOrganization)}
-      {suffix !== null && renderPlaceholderPart(suffix)}
     </View>
   );
 }

src/reactUtils.js

@@ -85,33 +85,22 @@ export const useHasStayedTrueForMs = (value: boolean, duration: number): boolean
 };
 
 /**
- * Invoke the callback as an effect when `value` turns from false to true.
+ * Like `useEffect`, but the callback only runs when `value` is true.
  *
- * This differs from putting `value` in a `useEffect` dependency list in
- * that the callback will *not* be invoked when the value changes in the
- * other direction, from true to false.
+ * Callers should wrap the callback in `useCallback` with an appropriate
+ * array of dependencies.
  *
- * `includeStart` controls what happens if `value` is true on the very first
- * render.  If `includeStart` is true, then the effect is triggered (so
- * treating its previous state of nonexistence as equivalent to being
- * false).  If `includeStart` is false, then the effect is not triggered (so
- * treating its previous state of nonexistence as not a valid state to
- * compare to.)
+ * The callback will run once at the beginning of every period of `value`
+ * being true, and again throughout such a period whenever the value of the
+ * callback changes.
  *
- * The callback is not permitted to return a cleanup function, because it's
- * not clear what the semantics should be of when such a cleanup function
- * would be run.
+ * As with `useEffect`, the cleanup function, if provided, will run once for
+ * every time the callback is called. If `value` goes from true to false,
+ * the cleanup function will be called at that time.
  */
-export const useEdgeTriggeredEffect = (
-  cb: () => void,
-  value: boolean,
-  includeStart: boolean,
-): void => {
-  const prev = usePrevious(value, !includeStart);
-  useEffect(() => {
-    if (value && !prev) {
-      cb();
-    }
-    // No dependencies list -- the effect itself decides whether to act.
-  });
-};
+// The claims about when `cb` runs assume that useEffect doesn't run its
+// callback on non-initial renders where its dependencies are unchanged. The
+// docs could be clearer about that:
+//   https://reactjs.org/docs/hooks-effect.html#tip-optimizing-performance-by-skipping-effects
+export const useConditionalEffect = (cb: () => void | (() => void), value: boolean): void =>
+  useEffect(() => (value ? cb() : undefined), [value, cb]);

src/start/RealmInputScreen.js

@@ -27,6 +27,14 @@ type State = {|
   progress: boolean,
 |};
 
+const urlFromInputValue = (realmInputValue: string): URL | void => {
+  const withScheme = /^https?:\/\//.test(realmInputValue)
+    ? realmInputValue
+    : `https://${realmInputValue}`;
+
+  return tryParseUrl(withScheme);
+};
+
 export default class RealmInputScreen extends PureComponent<Props, State> {
   state: State = {
     progress: false,
@@ -37,7 +45,7 @@ export default class RealmInputScreen extends PureComponent<Props, State> {
   tryRealm: () => Promise<void> = async () => {
     const { realmInputValue } = this.state;
 
-    const parsedRealm = tryParseUrl(realmInputValue);
+    const parsedRealm = urlFromInputValue(realmInputValue);
     if (!parsedRealm) {
       this.setState({ error: 'Please enter a valid URL' });
       return;
@@ -92,9 +100,6 @@ export default class RealmInputScreen extends PureComponent<Props, State> {
         <SmartUrlInput
           style={styles.input}
           navigation={navigation}
-          defaultProtocol="https://"
-          defaultOrganization="your-org"
-          defaultDomain="zulipchat.com"
           onChangeText={this.handleRealmChange}
           onSubmitEditing={this.tryRealm}
           enablesReturnKeyAutomatically
@@ -109,7 +114,7 @@ export default class RealmInputScreen extends PureComponent<Props, State> {
           text="Enter"
           progress={progress}
           onPress={this.tryRealm}
-          disabled={tryParseUrl(realmInputValue) === undefined}
+          disabled={urlFromInputValue(realmInputValue) === undefined}
         />
       </Screen>
     );