Merge pull request #5858 from ian-r-rose/completer-feature-parity

Completer feature parity

packages/completer/src/handler.ts

@@ -93,6 +93,7 @@ export class CompletionHandler implements IDisposable {
       const model = editor.model;
 
       editor.host.classList.remove(COMPLETER_ENABLED_CLASS);
+      editor.host.classList.remove(COMPLETER_ACTIVE_CLASS);
       model.selections.changed.disconnect(this.onSelectionsChanged, this);
       model.value.changed.disconnect(this.onTextChanged, this);
     }
@@ -261,6 +262,12 @@ export class CompletionHandler implements IDisposable {
       return;
     }
 
+    // If we are currently performing a subset match,
+    // return without resetting the completer.
+    if (model.subsetMatch) {
+      return;
+    }
+
     const position = editor.getCursorPosition();
     const line = editor.getLine(position.line);
     if (!line) {

packages/completer/src/model.ts

@@ -233,7 +233,6 @@ export class CompleterModel implements Completer.IModel {
       this._options = values;
       this._typeMap = types;
       this._orderedTypes = Private.findOrderedTypes(types);
-      this._subsetMatch = true;
     } else {
       this._options = [];
       this._typeMap = {};
@@ -252,7 +251,7 @@ export class CompleterModel implements Completer.IModel {
     }
 
     const { column, line } = change;
-    const { original } = this;
+    const { current, original } = this;
 
     if (!original) {
       return;
@@ -266,13 +265,13 @@ export class CompleterModel implements Completer.IModel {
     }
 
     // If a cursor change results in the cursor being set to a position that
-    // precedes the original request, cancel.
+    // precedes the original column, cancel.
     if (column < original.column) {
       this.reset(true);
       return;
     }
 
-    const { cursor, current } = this;
+    const { cursor } = this;
 
     if (!cursor || !current) {
       return;
@@ -358,7 +357,6 @@ export class CompleterModel implements Completer.IModel {
     if (!hard && this._subsetMatch) {
       return;
     }
-    this._subsetMatch = false;
     this._reset();
     this._stateChanged.emit(undefined);
   }

packages/completer/src/widget.ts

@@ -241,13 +241,6 @@ export class Completer extends Widget {
       return;
     }
 
-    // If there is only one item, signal and bail.
-    if (items.length === 1) {
-      this._selected.emit(items[0].raw);
-      this.reset();
-      return;
-    }
-
     // Clear the node.
     let node = this.node;
     node.textContent = '';
@@ -271,10 +264,8 @@ export class Completer extends Widget {
 
     // If this is the first time the current completer session has loaded,
     // populate any initial subset match.
-    if (model.subsetMatch) {
+    if (!model.query) {
       const populated = this._populateSubset();
-
-      model.subsetMatch = false;
       if (populated) {
         this.update();
         return;
@@ -298,16 +289,35 @@ export class Completer extends Widget {
    * `down` cycles will remain on the last index. When the user cycles `up` to
    * the first item, subsequent `up` cycles will remain on the first cycle.
    */
-  private _cycle(direction: 'up' | 'down'): void {
+  private _cycle(direction: Private.scrollType): void {
     let items = this.node.querySelectorAll(`.${ITEM_CLASS}`);
     let index = this._activeIndex;
     let active = this.node.querySelector(`.${ACTIVE_CLASS}`) as HTMLElement;
     active.classList.remove(ACTIVE_CLASS);
+
     if (direction === 'up') {
       this._activeIndex = index === 0 ? index : index - 1;
-    } else {
+    } else if (direction === 'down') {
       this._activeIndex = index < items.length - 1 ? index + 1 : index;
+    } else {
+      // Measure the number of items on a page.
+      const boxHeight = this.node.getBoundingClientRect().height;
+      const itemHeight = active.getBoundingClientRect().height;
+      const pageLength = Math.floor(boxHeight / itemHeight);
+
+      // Update the index
+      if (direction === 'pageUp') {
+        this._activeIndex = index - pageLength;
+      } else {
+        this._activeIndex = index + pageLength;
+      }
+      // Clamp to the length of the list.
+      this._activeIndex = Math.min(
+        Math.max(0, this._activeIndex),
+        items.length - 1
+      );
     }
+
     active = items[this._activeIndex] as HTMLElement;
     active.classList.add(ACTIVE_CLASS);
     ElementExt.scrollIntoViewIfNeeded(this.node, active);
@@ -333,13 +343,18 @@ export class Completer extends Widget {
         if (!model) {
           return;
         }
-        model.subsetMatch = true;
         let populated = this._populateSubset();
-        model.subsetMatch = false;
+        // If there is a common subset in the options,
+        // then emit a completion signal with that subset.
+        if (model.query) {
+          model.subsetMatch = true;
+          this._selected.emit(model.query);
+          model.subsetMatch = false;
+        }
+        // If the query changed, update rendering of the options.
         if (populated) {
-          return;
+          this.update();
         }
-        this.selectActive();
         return;
       case 27: // Esc key
         event.preventDefault();
@@ -347,12 +362,15 @@ export class Completer extends Widget {
         event.stopImmediatePropagation();
         this.reset();
         return;
+      case 33: // PageUp
+      case 34: // PageDown
       case 38: // Up arrow key
       case 40: // Down arrow key
         event.preventDefault();
         event.stopPropagation();
         event.stopImmediatePropagation();
-        this._cycle(event.keyCode === 38 ? 'up' : 'down');
+        const cycle = Private.keyCodeMap[event.keyCode];
+        this._cycle(cycle);
         return;
       default:
         return;
@@ -746,6 +764,21 @@ export namespace Completer {
  * A namespace for completer widget private data.
  */
 namespace Private {
+  /**
+   * Types of scrolling through the completer.
+   */
+  export type scrollType = 'up' | 'down' | 'pageUp' | 'pageDown';
+
+  /**
+   * Mapping from keyCodes to scrollTypes.
+   */
+  export const keyCodeMap: { [n: number]: scrollType } = {
+    38: 'up',
+    40: 'down',
+    33: 'pageUp',
+    34: 'pageDown'
+  };
+
   /**
    * Returns the common subset string that a list of strings shares.
    */

tests/test-completer/src/handler.spec.ts

@@ -122,6 +122,20 @@ describe('@jupyterlab/completer', () => {
         handler.editor = two.editor;
         expect(handler.editor).to.equal(two.editor);
       });
+
+      it('should remove the completer active and enabled classes of the old editor', () => {
+        const handler = new CompletionHandler({
+          connector,
+          completer: new Completer({ editor: null })
+        });
+        const widget = createEditorWidget();
+        handler.editor = widget.editor;
+        widget.toggleClass('jp-mod-completer-enabled');
+        widget.toggleClass('jp-mod-completer-active');
+        handler.editor = null;
+        expect(widget.hasClass('jp-mod-completer-enabled')).to.equal(false);
+        expect(widget.hasClass('jp-mod-completer-active')).to.equal(false);
+      });
     });
 
     describe('#isDisposed', () => {
@@ -186,7 +200,7 @@ describe('@jupyterlab/completer', () => {
         editor.setCursorPosition({ line: 0, column: 2 });
         // This signal is emitted (again) because the cursor position that
         // a natural user would create need to be recreated here.
-        (editor.model.value.changed as any).emit(void 0);
+        (editor.model.value.changed as any).emit({ type: 'set', value: 'bar' });
         expect(model.methods).to.contain('handleTextChange');
       });
     });

tests/test-completer/src/widget.spec.ts

@@ -122,7 +122,7 @@ describe('completer/widget', () => {
         Widget.attach(widget, document.body);
         MessageLoop.sendMessage(widget, Widget.Msg.UpdateRequest);
         expect(value).to.equal('');
-        simulate(anchor.node, 'keydown', { keyCode: 9 }); // Tab
+        widget.selectActive();
         expect(value).to.equal('foo');
         widget.dispose();
         anchor.dispose();
@@ -400,7 +400,7 @@ describe('completer/widget', () => {
           anchor.dispose();
         });
 
-        it('should mark common subset on start and select on tab', async () => {
+        it('should mark common subset on start and complete that subset on tab', async () => {
           let anchor = createEditorWidget();
           let model = new CompleterModel();
           let options: Completer.IOptions = {
@@ -465,6 +465,7 @@ describe('completer/widget', () => {
 
           let item = widget.node.querySelectorAll(`.${ITEM_CLASS} mark`)[1];
 
+          simulate(anchor.node, 'keydown', { keyCode: 9 }); // Tab key
           expect(model.query).to.equal('ba');
           simulate(item, 'mousedown');
           expect(value).to.equal('baz');
@@ -614,43 +615,6 @@ describe('completer/widget', () => {
     });
 
     describe('#onUpdateRequest()', () => {
-      it('should emit a selection if there is only one match', () => {
-        let anchor = createEditorWidget();
-        let model = new CompleterModel();
-        let coords = { left: 0, right: 0, top: 100, bottom: 120 };
-        let request: Completer.ITextState = {
-          column: 0,
-          lineHeight: 0,
-          charWidth: 0,
-          line: 0,
-          coords: coords as CodeEditor.ICoordinate,
-          text: 'f'
-        };
-
-        let value = '';
-        let options: Completer.IOptions = {
-          editor: anchor.editor,
-          model
-        };
-        let listener = (sender: any, selected: string) => {
-          value = selected;
-        };
-
-        Widget.attach(anchor, document.body);
-        model.original = request;
-        model.setOptions(['foo']);
-
-        let widget = new Completer(options);
-        widget.selected.connect(listener);
-        Widget.attach(widget, document.body);
-
-        expect(value).to.equal('');
-        MessageLoop.sendMessage(widget, Widget.Msg.UpdateRequest);
-        expect(value).to.equal('foo');
-        widget.dispose();
-        anchor.dispose();
-      });
-
       it('should do nothing if a model does not exist', () => {
         let widget = new LogWidget({ editor: null });
         MessageLoop.sendMessage(widget, Widget.Msg.UpdateRequest);