Trap the focus in the inline parameter editor popover (don't let tab outside of it)

Also fix some fields not allowing to press Escape to close

Don't show in changelog

newIDE/app/src/EventsSheet/InlinePopover.js

@@ -1,9 +1,14 @@
-import React, { Component } from 'react';
+// @flow
+import * as React from 'react';
 import Popper from '@material-ui/core/Popper';
 import Background from '../UI/Background';
 import ClickAwayListener from '@material-ui/core/ClickAwayListener';
 import { Column, Line } from '../UI/Grid';
-import { shouldCloseOrCancel } from '../UI/KeyboardShortcuts/InteractionKeys';
+import {
+  shouldCloseOrCancel,
+  shouldFocusNextField,
+  shouldFocusPreviousField,
+} from '../UI/KeyboardShortcuts/InteractionKeys';
 
 const styles = {
   popover: {
@@ -26,55 +31,94 @@ const styles = {
   },
 };
 
-export default class InlinePopover extends Component {
-  render() {
-    return (
-      <ClickAwayListener
-        onClickAway={event => {
-          if (event instanceof MouseEvent) {
-            // onClickAway is triggered on a "click" (which can actually happen
-            // on a touchscreen too!).
-            // The click already gave the opportunity to the popover content to
-            // get blurred (allowing "semi controlled" text fields
-            // to apply their changes). We can close now.
-            this.props.onRequestClose();
-          } else {
-            // Give a bit of time to the popover content to be blurred
-            // (useful for the "semi controlled" text fields for example)
-            // for touch events.
-            //
-            // This timeout needs to be at least around 50ms, otherwise
-            // blur events for GenericExpressionField are not triggered on iOS.
-            // There might be a better way to do this without waiting this much time.
-            setTimeout(() => {
-              this.props.onRequestClose();
-            }, 50);
+type Props = {|
+  children: React.Node,
+  anchorEl: ?HTMLElement,
+  open: boolean,
+  onRequestClose: () => void,
+|};
+
+/**
+ * A popover that can be used to show the field to edit a parameter, without
+ * opening the full instruction editor.
+ * Works like a dialog when opened (trapping the focus, dismissed on Escape,
+ * dismissed on click/touch outside) but positioned under the edited parameter.
+ */
+export default function InlinePopover(props: Props) {
+  const startSentinel = React.useRef<?HTMLDivElement>(null);
+  const endSentinel = React.useRef<?HTMLDivElement>(null);
+
+  return (
+    <ClickAwayListener
+      onClickAway={event => {
+        if (event instanceof MouseEvent) {
+          // onClickAway is triggered on a "click" (which can actually happen
+          // on a touchscreen too!).
+          // The click already gave the opportunity to the popover content to
+          // get blurred (allowing "semi controlled" text fields
+          // to apply their changes). We can close now.
+          props.onRequestClose();
+        } else {
+          // Give a bit of time to the popover content to be blurred
+          // (useful for the "semi controlled" text fields for example)
+          // for touch events.
+          //
+          // This timeout needs to be at least around 50ms, otherwise
+          // blur events for GenericExpressionField are not triggered on iOS.
+          // There might be a better way to do this without waiting this much time.
+          setTimeout(() => {
+            props.onRequestClose();
+          }, 50);
+        }
+      }}
+    >
+      <Popper
+        open={props.open}
+        anchorEl={props.anchorEl}
+        style={styles.popover}
+        placement="bottom-start"
+        onKeyDown={event => {
+          // Much like a dialog, offer a way to close the popover
+          // with a key.
+          // Note that the content of the popover can capture the event
+          // and stop its propagation (for example, the GenericExpressionField
+          // when showing autocompletion), which is fine.
+          if (shouldCloseOrCancel(event)) {
+            props.onRequestClose();
+          }
+
+          // Also like a dialog, add a "focus trap". If the user keeps pressing tab
+          // (or shift+tab), we "loop" the focus so that it stays inside the popover.
+          // Otherwise, the focus would espace and could go in some unrelated element
+          // in the events sheet, triggering a scroll, which would be very disturbing
+          // and would break the keyboard navigation.
+          if (shouldFocusNextField(event)) {
+            if (event.target && event.target === endSentinel.current) {
+              event.stopPropagation();
+              event.preventDefault();
+              if (startSentinel.current) {
+                startSentinel.current.focus();
+              }
+            }
+          } else if (shouldFocusPreviousField(event)) {
+            if (event.target && event.target === startSentinel.current) {
+              event.stopPropagation();
+              event.preventDefault();
+              if (endSentinel.current) {
+                endSentinel.current.focus();
+              }
+            }
           }
         }}
       >
-        <Popper
-          open={this.props.open}
-          anchorEl={this.props.anchorEl}
-          style={styles.popover}
-          placement="bottom"
-          onKeyDown={event => {
-            // Much like a dialog, offer a way to close the popover
-            // with a key.
-            // Note that the content of the popover can capture the event
-            // and stop its propagation (for example, the GenericExpressionField
-            // when showing autocompletion), which is fine.
-            if (shouldCloseOrCancel(event)) {
-              this.props.onRequestClose();
-            }
-          }}
-        >
-          <Background>
-            <Column expand>
-              <Line>{this.props.children}</Line>
-            </Column>
-          </Background>
-        </Popper>
-      </ClickAwayListener>
-    );
-  }
+        <Background>
+          <div tabIndex={0} ref={startSentinel} />
+          <Column expand>
+            <Line>{props.children}</Line>
+          </Column>
+          <div tabIndex={0} ref={endSentinel} />
+        </Background>
+      </Popper>
+    </ClickAwayListener>
+  );
 }

newIDE/app/src/EventsSheet/ParameterFields/AudioResourceField.js

@@ -42,6 +42,7 @@ export default class AudioResourceField extends Component<
         initialResourceName={this.props.value}
         onChange={this.props.onChange}
         floatingLabelText={<Trans>Choose the audio file to use</Trans>}
+        onRequestClose={this.props.onRequestClose}
         ref={field => (this._field = field)}
       />
     );

newIDE/app/src/EventsSheet/ParameterFields/JsonResourceField.js

@@ -42,6 +42,7 @@ export default class JsonResourceField extends Component<
         initialResourceName={this.props.value}
         onChange={this.props.onChange}
         floatingLabelText={<Trans>Choose the json file to use</Trans>}
+        onRequestClose={this.props.onRequestClose}
         ref={field => (this._field = field)}
       />
     );

newIDE/app/src/EventsSheet/ParameterFields/VideoResourceField.js

@@ -42,6 +42,7 @@ export default class VideoResourceField extends Component<
         initialResourceName={this.props.value}
         onChange={this.props.onChange}
         floatingLabelText={<Trans>Choose the video file to use</Trans>}
+        onRequestClose={this.props.onRequestClose}
         ref={field => (this._field = field)}
       />
     );

newIDE/app/src/ResourcesList/ResourceSelector.js

@@ -36,6 +36,7 @@ type Props = {|
   floatingLabelText?: React.Node,
   helperMarkdownText?: ?string,
   hintText?: MessageDescriptor,
+  onRequestClose?: () => void,
   margin?: 'none' | 'dense',
   style?: {| alignSelf?: 'center' |},
 |};
@@ -275,6 +276,7 @@ export default class ResourceSelector extends React.Component<Props, State> {
             errorText={errorText}
             fullWidth={this.props.fullWidth}
             margin={this.props.margin}
+            onRequestClose={this.props.onRequestClose}
             ref={autoComplete => (this._autoComplete = autoComplete)}
           />
         )}

newIDE/app/src/UI/KeyboardShortcuts/InteractionKeys.js

@@ -32,5 +32,14 @@ export const shouldActivate = (event: SupportedEvent) => {
  * (or material-ui), and using this should not be needed.
  */
 export const shouldFocusNextField = (event: SupportedEvent) => {
-  return event.key === 'Tab';
+  return event.key === 'Tab' && !event.shiftKey;
+};
+
+/**
+ * Check if the user asked to go to the previous field.
+ * Note that in most case, this should be automatically handled by the browser
+ * (or material-ui), and using this should not be needed.
+ */
+export const shouldFocusPreviousField = (event: SupportedEvent) => {
+  return event.key === 'Tab' && event.shiftKey;
 };