From c40a88b117727a85d7d581a0ff2e95334c95fd5a Mon Sep 17 00:00:00 2001
From: Benjamin Elo <beneloca@fb.com>
Date: Wed, 14 Aug 2019 05:24:39 -0700
Subject: [PATCH] Better form validation for required parameters plus live
 editing

Summary: Taking on the feedback from the demo yesterday, I've improved the required parameter's dialog by showing where specific errors occur in the form and adding live editing to the URI displayed.

Reviewed By: danielbuechele

Differential Revision: D16802921

fbshipit-source-id: 2e729549306a8efb79ca76d3da6f70632ccd9212
---
 .../components/RequiredParametersDialog.js    | 49 +++++++++++++------
 .../navigation/hooks/requiredParameters.js    |  9 +---
 src/plugins/navigation/index.js               |  7 +--
 src/plugins/navigation/util/uri.js            | 19 +++++++
 4 files changed, 57 insertions(+), 27 deletions(-)

diff --git a/src/plugins/navigation/components/RequiredParametersDialog.js b/src/plugins/navigation/components/RequiredParametersDialog.js
index 80298f0e7..6eb289b7e 100644
--- a/src/plugins/navigation/components/RequiredParametersDialog.js
+++ b/src/plugins/navigation/components/RequiredParametersDialog.js
@@ -7,7 +7,12 @@
  */
 
 import {Button, FlexColumn, Input, Sheet, styled, Glyph, colors} from 'flipper';
-import {replaceRequiredParametersWithValues} from '../util/uri';
+import {
+  replaceRequiredParametersWithValues,
+  parameterIsNumberType,
+  validateParameter,
+  liveEdit,
+} from '../util/uri';
 import {useRequiredParameterFormValidator} from '../hooks/requiredParameters';
 
 import type {URI} from '../flow-types';
@@ -36,10 +41,16 @@ const Text = styled('span')({
   lineHeight: 1.3,
 });
 
+const ErrorLabel = styled('span')({
+  color: colors.yellow,
+  lineHeight: 1.4,
+});
+
 const URIContainer = styled('div')({
   lineHeight: 1.3,
   marginLeft: 2,
   marginBottom: 8,
+  marginTop: 10,
   overflowWrap: 'break-word',
 });
 
@@ -49,8 +60,9 @@ const ButtonContainer = styled('div')({
 
 const RequiredParameterInput = styled(Input)({
   margin: 0,
-  marginBottom: 10,
+  marginTop: 8,
   height: 30,
+  width: '100%',
 });
 
 const WarningIconContainer = styled('span')({
@@ -81,24 +93,29 @@ export default (props: Props) => {
                 </WarningIconContainer>
                 <Text>
                   This uri has required parameters denoted by {'{parameter}'}.
-                  Numeric fields are spcified with a '#' symbol. Please fix the
-                  errors to navigate to the specified uri.
                 </Text>
               </Title>
               {requiredParameters.map((paramater, idx) => (
-                <RequiredParameterInput
-                  key={idx}
-                  onChange={event =>
-                    setValuesArray([
-                      ...values.slice(0, idx),
-                      event.target.value,
-                      ...values.slice(idx + 1),
-                    ])
-                  }
-                  placeholder={paramater}
-                />
+                <div key={idx}>
+                  <RequiredParameterInput
+                    onChange={event =>
+                      setValuesArray([
+                        ...values.slice(0, idx),
+                        event.target.value,
+                        ...values.slice(idx + 1),
+                      ])
+                    }
+                    name={paramater}
+                    placeholder={paramater}
+                  />
+                  {values[idx] &&
+                  parameterIsNumberType(paramater) &&
+                  !validateParameter(values[idx], paramater) ? (
+                    <ErrorLabel>Parameter must be a number</ErrorLabel>
+                  ) : null}
+                </div>
               ))}
-              <URIContainer>{uri}</URIContainer>
+              <URIContainer>{liveEdit(uri, values)}</URIContainer>
               <ButtonContainer>
                 <Button
                   onClick={() => {
diff --git a/src/plugins/navigation/hooks/requiredParameters.js b/src/plugins/navigation/hooks/requiredParameters.js
index 90031c56c..4865cb81a 100644
--- a/src/plugins/navigation/hooks/requiredParameters.js
+++ b/src/plugins/navigation/hooks/requiredParameters.js
@@ -7,14 +7,7 @@
  */
 
 import {useEffect, useState} from 'react';
-import {parameterIsNumberType} from '../util/uri';
-
-const validateParameter = (value: string, parameter: string) => {
-  return (
-    value.length > 0 &&
-    (parameterIsNumberType(parameter) ? !isNaN(parseInt(value, 10)) : true)
-  );
-};
+import {validateParameter} from '../util/uri';
 
 export const useRequiredParameterFormValidator = (
   requiredParameters: Array<string>,
diff --git a/src/plugins/navigation/index.js b/src/plugins/navigation/index.js
index 4882e0903..227f527d0 100644
--- a/src/plugins/navigation/index.js
+++ b/src/plugins/navigation/index.js
@@ -112,11 +112,12 @@ export default class extends FlipperPlugin<State, {}, PersistedState> {
   };
 
   navigateTo = (query: string) => {
-    this.props.setPersistedState({currentURI: query});
-    const requiredParameters = getRequiredParameters(query);
+    const filteredQuery = filterOptionalParameters(query);
+    this.props.setPersistedState({currentURI: filteredQuery});
+    const requiredParameters = getRequiredParameters(filteredQuery);
     if (requiredParameters.length === 0) {
       this.getDevice().then(device => {
-        device.navigateToLocation(filterOptionalParameters(query));
+        device.navigateToLocation(filterOptionalParameters(filteredQuery));
       });
     } else {
       this.setState({
diff --git a/src/plugins/navigation/util/uri.js b/src/plugins/navigation/util/uri.js
index 22a939987..c61a34ef5 100644
--- a/src/plugins/navigation/util/uri.js
+++ b/src/plugins/navigation/util/uri.js
@@ -8,6 +8,13 @@
 
 import querystring from 'querystring';
 
+export const validateParameter = (value: string, parameter: string) => {
+  return (
+    value &&
+    (parameterIsNumberType(parameter) ? !isNaN(parseInt(value, 10)) : true)
+  );
+};
+
 export const filterOptionalParameters: string => string = (uri: string) => {
   return uri.replace(/[/&]?([^&?={}\/]*=)?{\?.*?}/g, '');
 };
@@ -62,3 +69,15 @@ export const getRequiredParameters = (uri: string) => {
   }
   return matches;
 };
+
+export const liveEdit = (uri: string, formValues: Array<string>): string => {
+  const parameterRegExp = /({[^?]*?})/g;
+  const uriArray = uri.split(parameterRegExp);
+  return uriArray.reduce((acc, uriComponent, idx) => {
+    if (idx % 2 === 0 || !formValues[(idx - 1) / 2]) {
+      return acc + uriComponent;
+    } else {
+      return acc + formValues[(idx - 1) / 2];
+    }
+  });
+};