Fix iPad toolbox interactions; unify Monaco toolbox on the click path

theme/toolbox.less

@@ -31,6 +31,10 @@ span.blocklyTreeIcon {
     cursor: inherit;
 }
 
+.blocklyTreeInner {
+    user-select: none;
+}
+
 .blocklyToolbox.blocklyToolboxDeleting {
     background: var(--pxt-colors-red-background) !important;
     filter: brightness(1.2) saturate(0.8);

webapp/src/monaco.tsx

@@ -2003,10 +2003,12 @@ export class Editor extends toolboxeditor.ToolboxEditor {
     public onToolboxBlur(e: React.FocusEvent, hasSearch: boolean): void {
         const searchInputFocused = e.relatedTarget === (this.toolbox.refs.searchbox as toolbox.ToolboxSearch).refs.searchInput;
         const flyoutFocused = e.relatedTarget === this.flyout.refs.flyout || (this.flyout.refs.flyout as HTMLElement).contains(e.relatedTarget);
-        if (((searchInputFocused && !hasSearch) || !searchInputFocused) && !flyoutFocused) {
+        const tree = this.toolbox.refs.categoryTree as HTMLElement;
+        const treeFocused = !!tree && (e.relatedTarget === tree || tree.contains(e.relatedTarget as Node));
+        if (((searchInputFocused && !hasSearch) || !searchInputFocused) && !flyoutFocused && !treeFocused) {
             this.hideFlyout();
         }
-        if (!flyoutFocused) {
+        if (!flyoutFocused && !treeFocused) {
             this.toolbox.clear();
             this.toolbox.clearExpandedItem();
         }

webapp/src/monacoFlyout.tsx

@@ -184,6 +184,12 @@ export class MonacoFlyout extends data.Component<MonacoFlyoutProps, MonacoFlyout
             const isRtl = pxt.Util.isUserLanguageRtl();
             const charCode = core.keyCodeFromEvent(e);
             const target = e.target as HTMLElement;
+            const handledKey = charCode == 40 || charCode == 38 || charCode == 37
+                || charCode == 27 || (charCode == 13 && !!block);
+            if (handledKey) {
+                e.preventDefault();
+                e.stopPropagation();
+            }
             if (charCode == 40) { //  DOWN
                 // Next item
                 let nextSibling = target.nextElementSibling as HTMLElement;

webapp/src/toolbox.tsx

@@ -123,6 +123,11 @@ export interface ToolboxState {
 
 const MONACO_EDITOR_NAME: string = "monaco";
 
+// Scoped to the editor so the blocks and Monaco toolboxes have unique ids.
+function getToolboxItemId(editorname: string, nameid: string, subns?: string): string {
+    return `${editorname}-${nameid}${subns ?? ""}`;
+}
+
 export class Toolbox extends data.Component<ToolboxProps, ToolboxState> {
     private rootElement: HTMLElement;
 
@@ -531,45 +536,11 @@ export class Toolbox extends data.Component<ToolboxProps, ToolboxState> {
         this.props.parent.onToolboxBlur(e, this.state.hasSearch);
     }
 
-    handlePointerDownCapture = (e: React.PointerEvent) => {
-        e.preventDefault();
+    handlePointerDownCapture = () => {
         // A pointer tap focuses the tree, which would make handleCategoryTreeFocus
-        // auto-select the remembered category. On touch that focus event can arrive
-        // asynchronously after pointerup (and before the click), so keep focus
-        // handling disabled for the whole gesture until the final click.
+        // auto-select the remembered category, so keep focus handling disabled for
+        // the gesture; onCategoryClick (on the click) and handleKeyDown re-arm it.
         this.shouldHandleCategoryTreeFocus = false;
-        (this.refs.categoryTree as HTMLElement).focus();
-    }
-
-    handlePointerUp = (e: React.PointerEvent) => {
-        // On iOS Safari the *first* toolbox tap after Monaco's textarea has
-        // focus produces no synthesized mousedown/click. There's no clear cause
-        // (doesn't seem to be preventDefault, target element type, the manual
-        // focus call, or user-select) so we use pointerup for touch.
-        if (e.pointerType === "mouse" || this.props.editorname !== MONACO_EDITOR_NAME) return;
-
-        const target = e.target as HTMLElement;
-        const treeRow = target.closest(".blocklyTreeRow") as HTMLElement;
-        if (!treeRow) return;
-
-        const treeItem = treeRow.closest("[role='treeitem']") as HTMLElement;
-        if (!treeItem) return;
-
-        const id = treeItem.id;
-
-        // Handle the Advanced toggle button
-        if (id === "advanced") {
-            this.advancedClicked();
-            return;
-        }
-
-        for (const item of this.items) {
-            const itemId = item.subns ? item.nameid + item.subns : item.nameid;
-            if (itemId === id) {
-                this.onCategoryClick(item, this.items.indexOf(item));
-                return;
-            }
-        }
     }
 
     isRtl() {
@@ -578,6 +549,9 @@ export class Toolbox extends data.Component<ToolboxProps, ToolboxState> {
     }
 
     handleKeyDown(e: React.KeyboardEvent<HTMLElement>) {
+        // Keyboard use re-arms focus handling, which a touch gesture with no trailing
+        // click can leave disabled.
+        this.shouldHandleCategoryTreeFocus = true;
         // Take care to avoid default scroll behaviors and Blockly shortcuts running that overlap.
         const isRtl = Util.isUserLanguageRtl();
         const audioManager = (Blockly.getMainWorkspace() as Blockly.WorkspaceSvg)?.getAudioManager();
@@ -745,8 +719,7 @@ export class Toolbox extends data.Component<ToolboxProps, ToolboxState> {
                         onKeyDown={this.handleKeyDown}
                         // Prevents focus handling from running on pointer down events.
                         onPointerDownCapture={this.handlePointerDownCapture}
-                        onPointerUp={this.handlePointerUp}
-                        aria-activedescendant={selectedItem ? `${editorname}-${selectedItem}` : null}
+                        aria-activedescendant={selectedItem ? getToolboxItemId(editorname, selectedItem) : null}
                     >
                         {tryToDeleteNamespace &&
                             <DeleteConfirmationModal
@@ -927,7 +900,10 @@ export class CategoryItem extends data.Component<CategoryItemProps, CategoryItem
     }
 
     focusElement() {
-        this.treeRowElement.focus();
+        // preventScroll: a plain focus() scrolls ancestors to reveal the row, jerking
+        // the whole toolbox under the header; scrollElementIntoView below handles
+        // out-of-view rows.
+        this.treeRowElement.focus(true);
     }
 
     scrollElementIntoView(options: ScrollIntoViewOptions) {
@@ -957,7 +933,7 @@ export class CategoryItem extends data.Component<CategoryItemProps, CategoryItem
         const ariaExpanded = treeRow.subcategories ? isExpanded : undefined;
 
         return (
-            <TreeItem id={`${editorname}-${treeRow.nameid}${(treeRow.subns ?? "")}`} className={className} selected={selected} ariaHidden={ariaHidden} ariaLabel={ariaLabel} ariaLevel={ariaLevel} ariaExpanded={ariaExpanded} ariaHasPopup={ariaHasPopup}>
+            <TreeItem id={getToolboxItemId(editorname, treeRow.nameid, treeRow.subns)} className={className} selected={selected} ariaHidden={ariaHidden} ariaLabel={ariaLabel} ariaLevel={ariaLevel} ariaExpanded={ariaExpanded} ariaHasPopup={ariaHasPopup}>
                 <TreeRow
                     ref={this.handleTreeRowRef}
                     isRtl={toolbox.isRtl()}
@@ -1032,8 +1008,8 @@ export class TreeRow extends data.Component<TreeRowProps, {}> {
         this.handleDeleteClick = this.handleDeleteClick.bind(this);
     }
 
-    focus() {
-        if (this.treeRow) this.treeRow.focus();
+    focus(preventScroll = false) {
+        if (this.treeRow) this.treeRow.focus({ preventScroll });
     }
 
     scrollIntoView(options: ScrollIntoViewOptions) {
@@ -1125,7 +1101,7 @@ export class TreeRow extends data.Component<TreeRowProps, {}> {
                     >
                         {iconContent}
                     </span>
-                    <span id={`${editorname}-${nameid}${subns ?? ""}.label`} className="blocklyTreeLabel">
+                    <span id={`${getToolboxItemId(editorname, nameid, subns)}.label`} className="blocklyTreeLabel">
                         {rowTitle}
                     </span>
                     {hasDeleteButton &&