fix: picker attributes always true (#6773)

# Pull Request ## 📖 Description As described in #6311 picker's "filter-selected" and "filter-query" attributes were incorrectly configured and always ended up being true unless set via the prop. This change resolves this issue by: - remove "filter-query" and "filter-selected" attributes (never worked) - introduces new `disableQueryFilter` and `disableSelectionFilter` props and associated attributes, defaults to false to match current default behavior - deprecates the `filterQuery` and `filterSelected` props but keeps them in sync with the new props so we shouldn't break anyone who is setting these This change also fixes an issue where the "no options available" display wasn't always showing up: ![image](https://github.com/microsoft/fast/assets/7649425/0702e75d-893a-494f-8723-d0ed1095582f) When initialized with no options picker could open the menu without displaying the "no options available" message. This pr simplifies the mechanism that shows that to ensure it shows up when needed. ### 🎫 Issues closes #6311 ## ✅ Checklist ### General - [x] I have included a change request file using `$ yarn change` - [ ] I have added tests for my changes. - [x] I have tested my changes. - [ ] I have updated the project documentation to reflect my changes. - [x] I have read the [CONTRIBUTING](https://github.com/microsoft/fast/blob/master/CONTRIBUTING.md) documentation and followed the [standards](/docs/community/code-of-conduct/#our-standards) for this project. ### Component-specific - [ ] I have added a new component - [x] I have modified an existing component - [ ] I have updated the [definition file](https://github.com/microsoft/fast/blob/master/packages/web-components/fast-components/CONTRIBUTING.md#definition) - [ ] I have updated the [configuration file](https://github.com/microsoft/fast/blob/master/packages/web-components/fast-components/CONTRIBUTING.md#configuration)
microsoft · Jun 14, 2024 · e499c03 · e499c03
1 parent 3316428
commit e499c03
Show file tree

Hide file tree

Showing 7 changed files with 112 additions and 59 deletions.
diff --git a/change/@microsoft-fast-foundation-1959881c-668e-47f9-8672-2b3d6d278f72.json b/change/@microsoft-fast-foundation-1959881c-668e-47f9-8672-2b3d6d278f72.json
@@ -0,0 +1,7 @@
+{
+  "type": "prerelease",
+  "comment": "no options missing",
+  "packageName": "@microsoft/fast-foundation",
+  "email": "stephcomeau@msn.com",
+  "dependentChangeType": "prerelease"
+}
diff --git a/packages/web-components/fast-foundation/docs/api-report.md b/packages/web-components/fast-foundation/docs/api-report.md
@@ -1541,14 +1541,24 @@ export class FASTPicker extends FormAssociatedPicker {
     disabled: boolean;
     // (undocumented)
     disabledChanged(previous: boolean, next: boolean): void;
+    disableQueryFilter: boolean;
+    // (undocumented)
+    protected disableQueryFilterChanged(): void;
+    disableSelectionFilter: boolean;
+    // (undocumented)
+    protected disableSelectionFilterChanged(): void;
     // (undocumented)
     disconnectedCallback(): void;
     // @internal
     filteredOptionsList: string[];
-    // (undocumented)
-    protected filteredOptionsListChanged(): void;
+    // @deprecated
     filterQuery: boolean;
+    // (undocumented)
+    protected filterQueryChanged(): void;
+    // @deprecated
     filterSelected: boolean;
+    // (undocumented)
+    protected filterSelectedChanged(): void;
     // @internal
     flyoutOpen: boolean;
     // (undocumented)

diff --git a/packages/web-components/fast-foundation/src/picker/README.md b/packages/web-components/fast-foundation/src/picker/README.md
@@ -208,8 +208,10 @@ export class FASTTextField extends TextField {}
 | ---------------------------- | ------- | --------------------------- | ---------------------------- | ----------------------------------------------------------------------------------------------------------------------------- | -------------------- |
 | `selection`                  | public  | `string`                    | `""`                         | Currently selected items. Comma delineated string ie. "apples,oranges".                                                       |                      |
 | `options`                    | public  | `string`                    |                              | Currently available options. Comma delineated string ie. "apples,oranges".                                                    |                      |
-| `filterSelected`             | public  | `boolean`                   | `true`                       | Whether the component should remove an option from the list when it is in the selection                                       |                      |
-| `filterQuery`                | public  | `boolean`                   | `true`                       | Whether the component should remove options based on the current query                                                        |                      |
+| `filterSelected`             | public  | `boolean`                   | `false`                      | Indicates whether the component should remove an option from the list when it is in the selection.                            |                      |
+| `disableSelectionFilter`     | public  | `boolean`                   | `false`                      | Indicates whether the component should remove an option from the list when it is in the selection.                            |                      |
+| `filterQuery`                | public  | `boolean`                   | `true`                       | Indicates whether the component should remove options based on the current query.                                             |                      |
+| `disableQueryFilter`         | public  | `boolean`                   | `false`                      | Indicates whether the component should remove options based on the current query.                                             |                      |
 | `maxSelected`                | public  | `number or null`            | `null`                       | The maximum number of items that can be selected.                                                                             |                      |
 | `noSuggestionsText`          | public  | `string`                    | `"No suggestions available"` | The text to present to assistive technolgies when no suggestions are available.                                               |                      |
 | `suggestionsAvailableText`   | public  | `string`                    | `"Suggestions available"`    | The text to present to assistive technolgies when suggestions are available.                                                  |                      |
@@ -237,6 +239,10 @@ export class FASTTextField extends TextField {}
 | ---------------------------------- | --------- | ------------------------------------------------------------- | ---------------------------------- | --------- | -------------- |
 | `selectionChanged`                 | protected |                                                               |                                    | `void`    |                |
 | `optionsChanged`                   | protected |                                                               |                                    | `void`    |                |
+| `filterSelectedChanged`            | protected |                                                               |                                    | `void`    |                |
+| `disableSelectionFilterChanged`    | protected |                                                               |                                    | `void`    |                |
+| `filterQueryChanged`               | protected |                                                               |                                    | `void`    |                |
+| `disableQueryFilterChanged`        | protected |                                                               |                                    | `void`    |                |
 | `disabledChanged`                  | public    |                                                               | `previous: boolean, next: boolean` | `void`    |                |
 | `menuPlacementChanged`             | protected |                                                               |                                    | `void`    |                |
 | `showLoadingChanged`               | protected |                                                               |                                    | `void`    |                |
@@ -245,7 +251,6 @@ export class FASTTextField extends TextField {}
 | `menuOptionTemplateChanged`        | protected |                                                               |                                    | `void`    |                |
 | `defaultMenuOptionTemplateChanged` | protected |                                                               |                                    | `void`    |                |
 | `queryChanged`                     | protected |                                                               |                                    | `void`    |                |
-| `filteredOptionsListChanged`       | protected |                                                               |                                    | `void`    |                |
 | `flyoutOpenChanged`                | protected |                                                               |                                    | `void`    |                |
 | `focus`                            | public    | Move focus to the input element                               |                                    |           |                |
 | `handleKeyDown`                    | public    | Handle key down events.                                       | `e: KeyboardEvent`                 | `boolean` |                |
@@ -262,8 +267,8 @@ export class FASTTextField extends TextField {}
 | ---------------------------- | ------------------------ | -------------- |
 | `selection`                  | selection                |                |
 | `options`                    | options                  |                |
-| `filter-selected`            | filterSelected           |                |
-| `filter-query`               | filterQuery              |                |
+| `disable-selection-filter`   | disableSelectionFilter   |                |
+| `disable-query-filter`       | disableQueryFilter       |                |
 | `max-selected`               | maxSelected              |                |
 | `no-suggestions-text`        | noSuggestionsText        |                |
 | `suggestions-available-text` | suggestionsAvailableText |                |

diff --git a/packages/web-components/fast-foundation/src/picker/picker.spec.md b/packages/web-components/fast-foundation/src/picker/picker.spec.md
@@ -49,8 +49,8 @@ Picker is the top level container which hosts both a `picker-list` component to
 - `label`: The text applied to the `aria-label` attribute of the internal input element.
 - `labelledby`: The text applied to the `aria-labelledby` attribute of the internal input element.  
 - `placeholder`: The text used as the `placeholder` value for the internal input element.
-- `filter-selected`:  Whether to remove selected elements from the option list (default=true)
-- `filter-query`:  Whether to remove elements that don't match the query string (default=true)
+- `filter-selected`:  Whether to remove selected elements from the option list (default=false)
+- `disable-query-filter`:  Whether to remove elements that don't match the query string (default=false)
 - `menu-placement`: Controls the placement of the menu relative to the input element.
 (default="bottom-fill")
 

diff --git a/packages/web-components/fast-foundation/src/picker/picker.ts b/packages/web-components/fast-foundation/src/picker/picker.ts
@@ -100,22 +100,62 @@ export class FASTPicker extends FormAssociatedPicker {
     }
 
     /**
-     * Whether the component should remove an option from the list when it is in the selection
+     * Indicates whether the component should remove an option from the list when it is in the selection.
      *
-     * @remarks
-     * HTML Attribute: filter-selected
+     * @deprecated - use `Picker.disableSelectionFilter`.
      */
-    @attr({ attribute: "filter-selected", mode: "boolean" })
-    public filterSelected: boolean = true;
+    @observable
+    public filterSelected: boolean = false;
+    protected filterSelectedChanged(): void {
+        // keep the main prop in sync
+        if (this.disableSelectionFilter === this.filterSelected) {
+            this.disableSelectionFilter = !this.filterSelected;
+        }
+    }
 
     /**
-     * Whether the component should remove options based on the current query
+     * Indicates whether the component should remove an option from the list when it is in the selection.
      *
      * @remarks
-     * HTML Attribute: filter-query
+     * HTML Attribute: disable-selection-filter
      */
-    @attr({ attribute: "filter-query", mode: "boolean" })
+    @attr({ attribute: "disable-selection-filter", mode: "boolean" })
+    public disableSelectionFilter: boolean = false;
+    protected disableSelectionFilterChanged(): void {
+        // keep depracated prop in sync
+        if (this.disableSelectionFilter === this.filterQuery) {
+            this.filterSelected = !this.disableSelectionFilter;
+        }
+    }
+
+    /**
+     * Indicates whether the component should remove options based on the current query.
+     *
+     * @deprecated - use `Picker.disableQueryFilter`.
+     */
+    @observable
     public filterQuery: boolean = true;
+    protected filterQueryChanged(): void {
+        // keep the main prop in sync
+        if (this.disableQueryFilter === this.filterQuery) {
+            this.disableQueryFilter = !this.filterQuery;
+        }
+    }
+
+    /**
+     * Indicates whether the component should remove options based on the current query.
+     *
+     * @remarks
+     * HTML Attribute: disable-query-filter
+     */
+    @attr({ attribute: "disable-query-filter", mode: "boolean" })
+    public disableQueryFilter: boolean = false;
+    protected disableQueryFilterChanged(): void {
+        // keep depracated prop in sync
+        if (this.disableQueryFilter === this.filterQuery) {
+            this.filterQuery = !this.disableQueryFilter;
+        }
+    }
 
     /**
      * The maximum number of items that can be selected.
@@ -336,15 +376,6 @@ export class FASTPicker extends FormAssociatedPicker {
      */
     @observable
     public filteredOptionsList: string[] = [];
-    protected filteredOptionsListChanged(): void {
-        if (this.$fastController.isConnected) {
-            Updates.enqueue(() => {
-                this.showNoOptions =
-                    this.menuElement.querySelectorAll('[role="option"]').length === 0;
-                this.setFocusedOption(this.showNoOptions ? -1 : 0);
-            });
-        }
-    }
 
     /**
      *  Indicates if the flyout menu is open or not
@@ -408,14 +439,7 @@ export class FASTPicker extends FormAssociatedPicker {
      * @internal
      */
     @observable
-    public showNoOptions: boolean = false;
-    private showNoOptionsChanged(): void {
-        if (this.$fastController.isConnected) {
-            Updates.enqueue(() => {
-                this.setFocusedOption(0);
-            });
-        }
-    }
+    public showNoOptions: boolean = true;
 
     /**
      *  The anchored region config to apply.
@@ -585,13 +609,6 @@ export class FASTPicker extends FormAssociatedPicker {
 
         if (open && getRootActiveElement(this) === this.inputElement) {
             this.flyoutOpen = open;
-            Updates.enqueue(() => {
-                if (this.menuElement !== undefined) {
-                    this.setFocusedOption(0);
-                } else {
-                    this.disableMenu();
-                }
-            });
             return;
         }
 
@@ -621,12 +638,12 @@ export class FASTPicker extends FormAssociatedPicker {
     /**
      * Handle the menu options updated event from the child menu
      */
-    private handleMenuOptionsUpdated(e: Event): void {
+    private handleMenuOptionsUpdated = (e: Event): void => {
         e.preventDefault();
-        if (this.flyoutOpen) {
+        Updates.enqueue(() => {
             this.setFocusedOption(0);
-        }
-    }
+        });
+    };
 
     /**
      * Handle key down events.
@@ -809,7 +826,6 @@ export class FASTPicker extends FormAssociatedPicker {
      */
     public handleRegionLoaded(e: Event): void {
         Updates.enqueue(() => {
-            this.setFocusedOption(0);
             this.$emit("menuloaded", { bubbles: false });
         });
     }
@@ -966,6 +982,7 @@ export class FASTPicker extends FormAssociatedPicker {
      * Sets the currently focused menu option by index
      */
     private setFocusedOption(optionIndex: number): void {
+        this.updateNoOptions();
         if (
             !this.flyoutOpen ||
             optionIndex === -1 ||
@@ -1023,12 +1040,12 @@ export class FASTPicker extends FormAssociatedPicker {
      */
     private updateFilteredOptions(): void {
         this.filteredOptionsList = this.optionsList.slice(0);
-        if (this.filterSelected) {
+        if (!this.disableSelectionFilter) {
             this.filteredOptionsList = this.filteredOptionsList.filter(
                 el => this.selectedItems.indexOf(el) === -1
             );
         }
-        if (this.filterQuery && this.query !== "" && this.query !== undefined) {
+        if (!this.disableQueryFilter && this.query !== "" && this.query !== undefined) {
             // compare case-insensitive
             const filterQuery = this.query.toLowerCase();
             this.filteredOptionsList = this.filteredOptionsList.filter(
@@ -1067,4 +1084,8 @@ export class FASTPicker extends FormAssociatedPicker {
         "bottom-fill": FlyoutPosBottomFill,
         "tallest-fill": FlyoutPosTallestFill,
     };
+
+    private updateNoOptions(): void {
+        this.showNoOptions = this.menuElement.optionElements.length === 0;
+    }
 }
diff --git a/packages/web-components/fast-foundation/src/picker/stories/picker.register.ts b/packages/web-components/fast-foundation/src/picker/stories/picker.register.ts
@@ -35,6 +35,7 @@ const pickerStyles = css`
         min-height: calc(
             (var(--base-height-multiplier) + var(--density)) * var(--design-unit) * 1px
         );
+        box-sizing: border-box;
         display: flex;
         flex-direction: column;
         align-items: center;

diff --git a/packages/web-components/fast-foundation/src/picker/stories/picker.stories.ts b/packages/web-components/fast-foundation/src/picker/stories/picker.stories.ts
@@ -8,8 +8,9 @@ const storyTemplate = html<StoryArgs<FASTPicker>>`
     <fast-picker
         selection="${x => x.selection}"
         options="${x => x.options}"
-        ?filter-selected="${x => x.filterSelected}"
-        ?filter-query="${x => x.filterQuery}"
+        disable-selection-filter="${x => x.disableSelectionFilter}"
+        disable-query-filter="${x => x.disableQueryFilter}"
+        :showLoading="${x => x.showLoading}"
         ?disabled="${x => x.disabled}"
         max-selected="${x => x.maxSelected}"
         no-suggestions-text="${x => x.noSuggestionsText}"
@@ -24,16 +25,13 @@ const storyTemplate = html<StoryArgs<FASTPicker>>`
 
 export default {
     title: "Picker",
-    args: {
-        // TODO: These are always true https://github.com/microsoft/fast/issues/6311
-        filterQuery: true,
-        filterSelected: true,
-        disabled: false,
-    },
+    args: {},
     argTypes: {
-        filterQuery: { control: "boolean" },
-        filterSelected: { control: "boolean" },
+        queryFilterDisabled: { control: "boolean" },
         disabled: { control: "boolean" },
+        disableQueryFilter: { control: "boolean" },
+        disableSelectionFilter: { control: "boolean" },
+        showLoading: { control: "boolean" },
         label: { control: "text" },
         labelledBy: { control: "text" },
         loadingText: { control: "text" },
@@ -49,10 +47,21 @@ export const Picker: Story<FASTPicker> = renderComponent(storyTemplate).bind({})
 Picker.args = {
     label: "Fruit picker",
     loadingText: "Loading",
-    noSuggestionsText: "No such fruit",
+    noSuggestionsText: "No suggestions available",
     options: "apple, orange, banana, mango, strawberry, raspberry, blueberry",
     placeholder: "Choose fruit",
     selection: "apple",
     suggestionsAvailableText: "Found some fruit",
     disabled: false,
 };
+
+export const PickerEmpty: Story<FASTPicker> = renderComponent(storyTemplate).bind({});
+PickerEmpty.args = {
+    label: "Fruit picker",
+    loadingText: "Loading",
+    noSuggestionsText: "No suggestions available",
+    options: "",
+    placeholder: "Choose fruit",
+    selection: "",
+    suggestionsAvailableText: "Found some fruit",
+};