Fix five behavioural defects; two need a product decision

addon/components/attach/popover.js

@@ -13,7 +13,6 @@ export default class AttachPopoverComponent extends Component {
     @tracked hideDuration = 300;
     @tracked hideOn = 'mouseleave blur escapekey';
     @tracked interactive = false;
-    @tracked isOffset = false;
     @tracked isShown = false;
     @tracked lazyRender = false;
     @tracked modifiers = null;
@@ -192,7 +191,11 @@ export default class AttachPopoverComponent extends Component {
         }
 
         // If cursor is not on the attachment or target, hide the popover
-        if (!target.contains(event.target) && !(this.isOffset && this.isCursorBetweenTargetAndAttachment(event)) && this.floatingElement && !this.floatingElement.contains(event.target)) {
+        // NOTE: this used to read `!(this.isOffset && this.isCursorBetweenTargetAndAttachment(event))`.
+        // `isOffset` was never assigned from an argument or anywhere else, so it was permanently
+        // false — and `isCursorBetweenTargetAndAttachment` does not exist on this component, so
+        // had anything ever set the flag the popover would have thrown on every mousemove.
+        if (!target.contains(event.target) && this.floatingElement && !this.floatingElement.contains(event.target)) {
             // Remove this listener before hiding the attachment
             delete this.hideListenersOnDocumentByEvent.mousemove;
             document.removeEventListener('mousemove', this.hideIfMouseOutsideTargetOrAttachment, this.useCapture);

addon/components/filter/multi-option.js

@@ -38,14 +38,19 @@ export default class FilterMultiOptionComponent extends Component {
         }
     }
 
+    // power-select expects `@search` to RETURN the matches (or a promise of them). The previous
+    // version called `this.fetchOptions(...)` — a Task object, not a function, so the remote path
+    // threw — and on the local path assigned `this.options`, which both raised a backtracking
+    // assertion (it runs inside a modifier update) and permanently discarded every non-matching
+    // option, so clearing the query could not bring them back.
     @action search(query) {
         const { optionLabel, fetchUri, fetchParams = {} } = this.args;
 
         if (typeof fetchUri === 'string') {
-            return this.fetchOptions(fetchUri, { query, ...fetchParams });
+            return this.fetchOptions.perform(fetchUri, { query, ...fetchParams });
         }
 
-        this.options = this.options.filter((option) => {
+        return this.options.filter((option) => {
             const optionText = get(option, optionLabel ?? 'name') ?? option;
 
             if (typeof optionText === 'string') {
@@ -65,9 +70,15 @@ export default class FilterMultiOptionComponent extends Component {
         try {
             const options = yield this.fetch.get(uri, queryParams);
             this.options = options;
+
+            // Returned as well as stored: `search` hands this task's promise straight to
+            // power-select, which uses the resolved value as the result list.
+            return options;
         } catch (err) {
             debug('Error loading options: ' + err.message);
         }
+
+        return [];
     }
 
     parseValue(value) {

addon/components/modal.js

@@ -289,7 +289,9 @@ export default class Modal extends Component {
      * @readonly
      * @private
      */
-    @usesTransition('_fade') usesTransition;
+    // Must name the ARGUMENT: the decorator reads `this.args[prop]`, and `_fade` is a getter on
+    // the component, so `this.args._fade` was always undefined and `@fade={{false}}` was ignored.
+    @usesTransition('fade') usesTransition;
 
     destinationElement = getDestinationElement(this);
 

addon/components/overlay.js

@@ -189,16 +189,25 @@ export default class OverlayComponent extends Component {
         const height = dy * multiplier + this.overlayHeight;
         const minResizeWidth = this.args.minResizeWidth ?? 560;
         const maxResizeWidth = this.args.maxResizeWidth ?? 900;
-
-        // Min resize width
-        if (width <= minResizeWidth) {
-            overlayPanelNode.style.width = `${minResizeWidth}px`;
+        const minResizeHeight = this.args.minResizeHeight ?? 0;
+        const maxResizeHeight = this.args.maxResizeHeight ?? Number.MAX_SAFE_INTEGER;
+
+        // Clamp the dimension actually being dragged. These guards used to test the WIDTH whatever
+        // the position and `return` before the fork below, so a top/bottom overlay whose width sat
+        // outside [min, max] — which a full-width drawer always does — could never be resized at
+        // all, and each vertical drag silently rewrote its width to the clamp.
+        const size = isHorizontal ? width : height;
+        const minSize = isHorizontal ? minResizeWidth : minResizeHeight;
+        const maxSize = isHorizontal ? maxResizeWidth : maxResizeHeight;
+        const dimension = isHorizontal ? 'width' : 'height';
+
+        if (size <= minSize) {
+            overlayPanelNode.style[dimension] = `${minSize}px`;
             return;
         }
 
-        // Max resize width
-        if (width >= maxResizeWidth) {
-            overlayPanelNode.style.width = `${maxResizeWidth}px`;
+        if (size >= maxSize) {
+            overlayPanelNode.style[dimension] = `${maxSize}px`;
             return;
         }
 

addon/components/query-builder/conditions.hbs

@@ -1,4 +1,6 @@
-<div class="query-builder-panel" ...attributes>
+{{! Re-validate when the caller narrows the column list: without this the panel keeps
+     sorting/grouping/filtering by columns that are no longer selected. }}
+<div class="query-builder-panel" {{did-update this.validateConditions @allSelectedColumns @selectedColumns}} ...attributes>
     <div class="query-builder-panel-header">
         <div class="query-builder-panel-title">
             <FaIcon @icon="filter" @size="sm" class="mr-2" />

addon/components/query-builder/group-by.hbs

@@ -1,4 +1,6 @@
-<div class="query-builder-panel" ...attributes>
+{{! Re-validate when the caller narrows the column list: without this the panel keeps
+     sorting/grouping/filtering by columns that are no longer selected. }}
+<div class="query-builder-panel" {{did-update this.validateGroupByItems @selectedColumns}} ...attributes>
     <div class="query-builder-panel-header">
         <div class="query-builder-panel-title">
             <FaIcon @icon="layer-group" @size="sm" class="mr-2" />

addon/components/query-builder/sort-by.hbs

@@ -1,4 +1,6 @@
-<div class="query-builder-panel" ...attributes>
+{{! Re-validate when the caller narrows the column list: without this the panel keeps
+     sorting/grouping/filtering by columns that are no longer selected. }}
+<div class="query-builder-panel" {{did-update this.validateSortItems @allSelectedColumns @selectedColumns}} ...attributes>
     <div class="query-builder-panel-header">
         <div class="query-builder-panel-title">
             <FaIcon @icon="sort" @size="sm" class="mr-2" />

tests/integration/components/filter/multi-option-test.js

@@ -4,6 +4,7 @@ import { render, settled, findAll, find } from '@ember/test-helpers';
 import { hbs } from 'ember-cli-htmlbars';
 import Service from '@ember/service';
 import { selectChoose, getDropdownItems } from 'ember-power-select/test-support';
+import { clickTrigger, typeInSearch } from 'ember-power-select/test-support/helpers';
 
 const FILTER = { param: 'status', label: 'Status' };
 const OPTIONS = [
@@ -253,4 +254,75 @@ module('Integration | Component | filter/multi-option', function (hooks) {
             assert.deepEqual(requests, []);
         });
     });
+    module('searching', function (hooks) {
+        // The search box only renders when the filter definition asks for it.
+        hooks.beforeEach(function () {
+            this.set('filter', { ...FILTER, multiOptionSearchEnabled: true });
+        });
+
+        test('a local option list is narrowed by the query, and restored when it is cleared', async function (assert) {
+            this.set('options', OPTIONS);
+
+            await render(TEMPLATE);
+            await clickTrigger(SCOPE);
+            await typeInSearch(SCOPE, 'act');
+
+            assert.deepEqual(
+                findAll('.ember-power-select-option').map((node) => node.textContent.trim()),
+                ['Active'],
+                'only the matching option is offered'
+            );
+
+            await typeInSearch(SCOPE, '');
+
+            assert.deepEqual(
+                findAll('.ember-power-select-option').map((node) => node.textContent.trim()),
+                ['Active', 'Pending', 'Cancelled'],
+                'clearing the query brings every option back rather than losing them for good'
+            );
+        });
+
+        test('with no optionLabel the option itself is matched', async function (assert) {
+            this.set('options', ['active', 'pending']);
+            this.set('optionLabel', undefined);
+
+            await render(TEMPLATE);
+            await clickTrigger(SCOPE);
+            await typeInSearch(SCOPE, 'pend');
+
+            assert.deepEqual(
+                findAll('.ember-power-select-option').map((node) => node.textContent.trim()),
+                ['pending']
+            );
+        });
+
+        test('an option with no text to match on is dropped rather than throwing', async function (assert) {
+            this.set('options', [{ name: 'Active', id: 'active' }, { id: 7 }]);
+
+            await render(TEMPLATE);
+            await clickTrigger(SCOPE);
+            await typeInSearch(SCOPE, 'a');
+
+            assert.deepEqual(
+                findAll('.ember-power-select-option').map((node) => node.textContent.trim()),
+                ['Active'],
+                'the option with no name is filtered out'
+            );
+        });
+
+        test('with a fetch uri the query is sent to the server instead', async function (assert) {
+            this.set('fetchUri', 'statuses');
+            this.set('fetchParams', { scope: 'orders' });
+
+            await render(TEMPLATE);
+            assert.strictEqual(requests.length, 1, 'the initial load');
+
+            await clickTrigger(SCOPE);
+            await typeInSearch(SCOPE, 'act');
+
+            assert.strictEqual(requests.length, 2, 'typing searches remotely rather than filtering locally');
+            assert.strictEqual(requests[1].params.query, 'act', 'the query is forwarded');
+            assert.strictEqual(requests[1].params.scope, 'orders', 'alongside the standing fetch params');
+        });
+    });
 });

tests/integration/components/modal-test.js

@@ -230,4 +230,18 @@ module('Integration | Component | modal', function (hooks) {
             assert.strictEqual(dialog(), null, 'closing removes it again');
         });
     });
+    // The decorator reads `this.args[prop]`, so it had been pointed at the `_fade` getter and
+    // `this.args._fade` was always undefined — @fade={{false}} never disabled anything.
+    test('@fade={{false}} renders without the fade class', async function (assert) {
+        await render(hbs`<Modal @open={{true}} @fade={{false}} />`);
+
+        assert.dom('.flb--modal-backdrop').doesNotHaveClass('fade', 'the backdrop is not faded in');
+        assert.dom('.flb--modal-backdrop').hasClass('show', 'and is shown straight away');
+    });
+
+    test('the backdrop fades by default', async function (assert) {
+        await render(hbs`<Modal @open={{true}} />`);
+
+        assert.dom('.flb--modal-backdrop').hasClass('fade');
+    });
 });

tests/integration/components/overlay-test.js

@@ -393,4 +393,32 @@ module('Integration | Component | overlay', function (hooks) {
             mouse('mouseup', document, { clientX: 700 });
         });
     });
+    test('a bottom-positioned overlay resizes by height', async function (assert) {
+        await render(hbs`<Overlay @position="bottom" @isResizable={{true}} @onResize={{this.onResize}} />`);
+
+        const startingHeight = panel().getBoundingClientRect().height;
+
+        // Dragging a bottom panel upward grows it: the multiplier is inverted.
+        mouse('mousedown', gutter(), { clientY: 0 });
+        mouse('mousemove', document, { clientY: -700 });
+
+        assert.strictEqual(named('onResize').length, 1, 'the movement is reported');
+        assert.strictEqual(panel().style.height, `${startingHeight + 700}px`, 'the drag distance is added to the starting height');
+        assert.strictEqual(panel().style.width, '', 'and its width is left alone by a vertical drag');
+        assert.strictEqual(document.body.style.cursor, 'row-resize');
+
+        mouse('mouseup', document, { clientY: -700 });
+    });
+
+    test('a vertical resize clamps on height, not on width', async function (assert) {
+        await render(hbs`<Overlay @position="bottom" @isResizable={{true}} @maxResizeHeight={{400}} @onResize={{this.onResize}} />`);
+
+        mouse('mousedown', gutter(), { clientY: 0 });
+        mouse('mousemove', document, { clientY: -5000 });
+
+        assert.strictEqual(panel().style.height, '400px', 'the height clamp applies');
+        assert.strictEqual(panel().style.width, '', 'the width clamp does not fire on a vertical drag');
+
+        mouse('mouseup', document, { clientY: -5000 });
+    });
 });

tests/integration/components/query-builder/sort-by-test.js

@@ -1,6 +1,6 @@
 import { module, test } from 'qunit';
 import { setupRenderingTest } from 'dummy/tests/helpers';
-import { render, click, findAll, triggerEvent } from '@ember/test-helpers';
+import { render, click, findAll, settled, triggerEvent } from '@ember/test-helpers';
 import { hbs } from 'ember-cli-htmlbars';
 import { selectChoose, getDropdownItems } from 'ember-power-select/test-support';
 
@@ -334,4 +334,38 @@ module('Integration | Component | query-builder/sort-by', function (hooks) {
 
         assert.dom('.query-builder-panel').hasAttribute('data-test-sort-by', 'yes');
     });
+    // Regression: `validateSortItems` existed but nothing called it, so narrowing the column list
+    // left the panel sorting by a column that was no longer selected.
+    async function addSortByLabel(label) {
+        await selectChoose(COLUMN_SELECT, label);
+        await click(addButton());
+    }
+
+    test('narrowing the selected columns drops a sort that no longer applies', async function (assert) {
+        await render(TEMPLATE);
+        await addSortByLabel('Status');
+        await addSortByLabel('Total');
+
+        assert.strictEqual(sortItems().length, 2, 'both sorts are listed');
+
+        this.set('allSelectedColumns', [COLUMNS[1]]);
+        this.set('selectedColumns', [COLUMNS[1]]);
+        await settled();
+
+        assert.strictEqual(sortItems().length, 1, 'the sort on the removed column is dropped');
+        assert.true(itemText(0).includes('Total'), 'the surviving sort is the one still selected');
+        assert.strictEqual(changes[changes.length - 1].length, 1, 'and the change is reported');
+    });
+
+    test('removing every column clears the sorting', async function (assert) {
+        await render(TEMPLATE);
+        await addSortByLabel('Status');
+
+        this.set('allSelectedColumns', []);
+        this.set('selectedColumns', []);
+        await settled();
+
+        assert.strictEqual(sortItems().length, 0);
+        assert.deepEqual(changes[changes.length - 1], [], 'the empty list is reported');
+    });
 });