Merge pull request #81 from phorest/bug/listbox-prevent-document-scro…

…ll-if-active

🐛 `<Listbox>`: 4 bug fixes for scrolling & focus

addon/components/listbox.hbs

@@ -18,6 +18,7 @@
       unsetActiveOption=this.unsetActiveOption
       setSelectedOption=this.setSelectedOption
       handleKeyPress=this.handleKeyPress
+      handleKeyDown=this.handleKeyDown
       handleKeyUp=this.handleKeyUp
       openListbox=this.openListbox
       closeListbox=this.closeListbox
@@ -28,8 +29,10 @@
       guid=this.guid
       isOpen=this.isOpen
       registerButtonElement=this.registerButtonElement
+      unregisterButtonElement=this.unregisterButtonElement
       handleButtonClick=this.handleButtonClick
       handleKeyPress=this.handleKeyPress
+      handleKeyDown=this.handleKeyDown
       handleKeyUp=this.handleKeyUp
       isDisabled=this.isDisabled
       openListbox=this.openListbox

addon/components/listbox.js

@@ -8,6 +8,17 @@ const ACTIVATE_NONE = 0;
 const ACTIVATE_FIRST = 1;
 const ACTIVATE_LAST = 2;
 
+const PREVENTED_KEYDOWN_EVENTS = new Set([
+  'ArrowUp',
+  'ArrowDown',
+  'ArrowLeft',
+  'ArrowRight',
+  'PageUp',
+  'PageDown',
+  'Home',
+  'End',
+]);
+
 export default class ListboxComponent extends Component {
   @tracked activeOptionIndex;
   activateBehaviour = ACTIVATE_NONE;
@@ -74,6 +85,13 @@ export default class ListboxComponent extends Component {
     return true;
   }
 
+  @action
+  handleKeyDown(event) {
+    if (PREVENTED_KEYDOWN_EVENTS.has(event.key)) {
+      event.preventDefault();
+    }
+  }
+
   @action
   handleKeyUp(event) {
     if (event.key === 'ArrowDown') {
@@ -142,6 +160,11 @@ export default class ListboxComponent extends Component {
     this.buttonElement = buttonElement;
   }
 
+  @action
+  unregisterButtonElement() {
+    this.buttonElement = undefined;
+  }
+
   @action
   registerLabelElement(labelElement) {
     this.labelElement = labelElement;
@@ -161,6 +184,8 @@ export default class ListboxComponent extends Component {
       if (this.args.value === optionComponent.args.value) {
         this.selectedOptionIndex = this.activeOptionIndex =
           this.optionElements.length - 1;
+
+        this.scrollIntoView(optionElement);
       }
     }
 
@@ -223,6 +248,18 @@ export default class ListboxComponent extends Component {
     }
   }
 
+  scrollIntoView(optionElement) {
+    // Cannot use optionElement.scrollIntoView() here because that function
+    // also scrolls the *window* by some amount. Here, we don't want to
+    // jerk the window, we just want to make the the option element visible
+    // inside its container.
+
+    optionElement.parentElement.scroll(
+      0,
+      optionElement.offsetTop - optionElement.parentElement.offsetTop
+    );
+  }
+
   @action
   unregisterOptionsElement() {
     this.optionsElement = undefined;
@@ -283,13 +320,14 @@ export default class ListboxComponent extends Component {
     this.search += key.toLowerCase();
 
     for (let i = 0; i < this.optionElements.length; i++) {
+      let optionElement = this.optionElements[i];
+
       if (
-        !this.optionElements[i].hasAttribute('disabled') &&
-        this.optionElements[i].textContent
-          .trim()
-          .toLowerCase()
-          .startsWith(this.search)
+        !optionElement.hasAttribute('disabled') &&
+        optionElement.textContent.trim().toLowerCase().startsWith(this.search)
       ) {
+        this.scrollIntoView(optionElement);
+
         this.activeOptionIndex = i;
         break;
       }

addon/components/listbox/-button.hbs

@@ -1,16 +1,19 @@
 {{#let (element (or @as 'button')) as |Tag|}}
   <Tag
     role={{if @role @role 'button'}}
+    type={{if @type @type 'button'}}
     id='{{@guid}}-button'
     aria-haspopup='listbox'
     aria-controls={{@guid}}
     aria-labelledby='{{@guid}}-label {{@guid}}-button'
     {{on 'click' @handleButtonClick}}
+    {{on 'keydown' @handleKeyDown}}
     {{on 'keyup' @handleKeyUp}}
     {{on 'keypress' @handleKeyPress}}
     aria-expanded={{unless @isDisabled (if @isOpen 'true' 'false')}}
     disabled={{@isDisabled}}
     {{did-insert @registerButtonElement}}
+    {{will-destroy @unregisterButtonElement}}
     ...attributes
   >
     {{yield}}

addon/components/listbox/-options.hbs

@@ -14,6 +14,7 @@
       {{did-insert @registerOptionsElement}}
       {{will-destroy @unregisterOptionsElement}}
       {{on 'keypress' @handleKeyPress}}
+      {{on 'keydown' @handleKeyDown}}
       {{on 'keyup' @handleKeyUp}}
       {{headlessui-focus-trap
         focusTrapOptions=(hash

tests/dummy/app/components/listboxes/basic.hbs

@@ -30,7 +30,7 @@
       </span>
     </listbox.Button>
     <listbox.Options
-      class='z-30 max-w-md absolute w-full py-1 mt-1 overflow-auto text-base bg-white rounded-md shadow-lg max-h-60 ring-1 ring-black ring-opacity-5 focus:outline-none sm:text-sm'
+      class='z-30 max-w-md w-full py-1 mt-1 overflow-auto text-base bg-white rounded-md shadow-lg max-h-60 ring-1 ring-black ring-opacity-5 focus:outline-none sm:text-sm'
       as |options|
     >
       {{#each this.people as |person|}}

tests/integration/components/listbox-test.js

@@ -3382,4 +3382,33 @@ module('Integration | Component | <Listbox>', function (hooks) {
       assertNoActiveListboxOption();
     });
   });
+
+  test('should be possible to open a listbox without submitting the form', async function (assert) {
+    let callCount = 0;
+
+    this.set('onSubmit', () => {
+      callCount++;
+    });
+
+    await render(hbs`
+      <form {{on 'submit' this.onSubmit}}>
+        <Listbox as |listbox|>
+          <listbox.Button>Trigger</listbox.Button>
+          <listbox.Options as |options|>
+            <options.Option>option</options.Option>
+          </listbox.Options>
+        </Listbox>
+      </form>
+    `);
+    assertListboxButton({
+      state: ListboxState.InvisibleUnmounted,
+    });
+    assertListbox({
+      state: ListboxState.InvisibleUnmounted,
+    });
+    await click(getListboxButton());
+    assertListbox({ state: ListboxState.Visible });
+
+    assert.equal(callCount, 0, 'onSubmit not called');
+  });
 });