Merge pull request #5426 from ajaxorg/fix-popup

fix regression with popup selectedNode and improve types
ajaxorg · Dec 14, 2023 · 3c69933 · 3c69933
2 parents b303256 + 047f2ac
commit 3c69933
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 18 deletions.
diff --git a/ace-internal.d.ts b/ace-internal.d.ts
@@ -1199,6 +1199,12 @@ export const Range: {
 export type InlineAutocomplete = Ace.InlineAutocomplete;
 export type CommandBarTooltip = Ace.CommandBarTooltip;
 
+declare global {
+    interface Element {
+        setAttribute(name: string, value: boolean): void;
+        setAttribute(name: string, value: number): void;
+    }
+}
 
 declare module "./src/anchor" {
     export interface Anchor extends Ace.EventEmitter<Ace.AnchorEvents> {
@@ -1431,7 +1437,8 @@ declare module "./src/autocomplete/popup" {
         getTextLeftOffset: () => number,
         $imageSize: number,
         anchorPos: any,
-        isMouseOver?: boolean
+        isMouseOver?: boolean,
+        selectedNode?: HTMLElement,
     }
 }
 
@@ -1450,6 +1457,7 @@ declare module "./src/layer/gutter" {
 
 declare module "./src/layer/text" {
     export interface Text extends Ace.EventEmitter<Ace.TextEvents> {
+        config?: Ace.LayerConfig
     }
 }
 

diff --git a/src/autocomplete/popup.js b/src/autocomplete/popup.js
@@ -135,17 +135,16 @@ class AcePopup {
         });
         popup.renderer.on("afterRender", function () {
             var row = popup.getRow();
-            /**@type {any}*/
             var t = popup.renderer.$textLayer;
-            var selected = t.element.childNodes[row - t.config.firstRow];
+            var selected = /** @type {HTMLElement|null} */(t.element.childNodes[row - t.config.firstRow]);
             var el = document.activeElement; // Active element is textarea of main editor
-            if (selected !== t.selectedNode && t.selectedNode) {
-                dom.removeCssClass(t.selectedNode, "ace_selected");
+            if (selected !== popup.selectedNode && popup.selectedNode) {
+                dom.removeCssClass(popup.selectedNode, "ace_selected");
                 el.removeAttribute("aria-activedescendant");
-                selected.removeAttribute(ariaActiveState);
-                t.selectedNode.removeAttribute("id");
+                popup.selectedNode.removeAttribute(ariaActiveState);
+                popup.selectedNode.removeAttribute("id");
             }
-            t.selectedNode = selected;
+            popup.selectedNode = selected;
             if (selected) {
                 dom.addCssClass(selected, "ace_selected");
                 var ariaId = getAriaId(row);

diff --git a/src/autocomplete_test.js b/src/autocomplete_test.js
@@ -858,16 +858,15 @@ module.exports = {
         var inline = completer.inlineRenderer;
 
         // Popup should be open, with inline text renderered.
-        assert.equal(editor.completer.popup.isOpen, true);  
+        assert.equal(completer.popup.isOpen, true);  
         assert.equal(completer.popup.getRow(), 0);
         assert.strictEqual(inline.isOpen(), true);
         assert.strictEqual(editor.renderer.$ghostText.text, "function\nthat does something\ncool");
 
-        editor.completer.popup.renderer.$loop._flush();
-        var popupTextLayer = completer.popup.renderer.$textLayer;
+        completer.popup.renderer.$loop._flush();
 
         // aria-describedby of selected popup item should have aria-describedby set to the offscreen inline screen reader div and doc-tooltip.
-        assert.strictEqual(popupTextLayer.selectedNode.getAttribute("aria-describedby"), "doc-tooltip ace-inline-screenreader-line-0 ace-inline-screenreader-line-1 ace-inline-screenreader-line-2 ");
+        assert.strictEqual(completer.popup.selectedNode.getAttribute("aria-describedby"), "doc-tooltip ace-inline-screenreader-line-0 ace-inline-screenreader-line-1 ace-inline-screenreader-line-2 ");
 
         // The elements with these IDs should have the correct content.
         assert.strictEqual(document.getElementById("ace-inline-screenreader-line-0").textContent,"function");

diff --git a/src/layer/text.js b/src/layer/text.js
@@ -1,6 +1,6 @@
 "use strict";
 /**
- *
+ * @typedef {import("../../ace-internal").Ace.LayerConfig} LayerConfig
  * @typedef {import("../edit_session").EditSession} EditSession
  */
 var oop = require("../lib/oop");
@@ -108,7 +108,7 @@ class Text {
     }
 
     /**
-     * @param {any} display
+     * @param {boolean} display
      */
     setDisplayIndentGuides(display) {
         if (this.displayIndentGuides == display)
@@ -120,7 +120,7 @@ class Text {
     }
 
     /**
-     * @param {any} highlight
+     * @param {boolean} highlight
      */
     setHighlightIndentGuides(highlight) {
         if (this.$highlightIndentGuides === highlight) return false;
@@ -169,7 +169,7 @@ class Text {
     }
 
     /**
-     * @param {{ lastRow: number; firstRow: number; lineHeight: number; }} config
+     * @param {LayerConfig} config
      * @param {number} firstRow
      * @param {number} lastRow
      */
@@ -242,6 +242,9 @@ class Text {
         }
     }
 
+    /**
+     * @param {LayerConfig} config
+     */
     scrollLines(config) {
         var oldConfig = this.config;
         this.config = config;
@@ -284,6 +287,11 @@ class Text {
         this.$highlightIndentGuide();
     }
 
+    /**
+     * @param {LayerConfig} config
+     * @param {number} firstRow
+     * @param {number} lastRow
+     */
     $renderLinesFragment(config, firstRow, lastRow) {
         var fragment = [];
         var row = firstRow;
@@ -322,6 +330,9 @@ class Text {
         return fragment;
     }
 
+    /**
+     * @param {LayerConfig} config
+     */
     update(config) {
         this.$lines.moveContainer(config);
 
@@ -687,8 +698,8 @@ class Text {
     }
 
     /**
-     * @param {any} row
-     * @param {{ walk: (arg0: (placeholder: any, row: any, column: any, lastColumn: any, isNewRow: any) => void, arg1: any, arg2: any) => void; end: { row: any; }; }} foldLine
+     * @param {number} row
+     * @param {import("../../ace-internal").Ace.FoldLine} foldLine
      * @return {import("../../ace-internal").Ace.Token[]}
      */
     $getFoldLineTokens(row, foldLine) {