Skip to content

Commit

Permalink
fix select dropdown positioning (microsoft#5811)
Browse files Browse the repository at this point in the history 
* fix: fix positioning of select and combobox dropdown

fix: fix positioning of combobox dropdown

add docs changes

initialize position property

sort imports

remove default initializer for position attribute

make position property nullable

update api-report and README

* Change files

* make position attribute nullable

Co-authored-by: John Kreitlow <john.kreitlow@microsoft.com>
  • Loading branch information
@simonxabris @radium-v
simonxabris and radium-v authored May 4, 2022
1 parent 5b82a4b commit 10169e7
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 39 deletions.
7 changes: 7 additions & 0 deletions change/@microsoft-fast-foundation-22faaad5-04d8-4a4c-bb54-a2790225d32c.json
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "fix positioning of select dropdown",
"packageName": "@microsoft/fast-foundation",
"email": "abris96@gmail.com",
"dependentChangeType": "patch"
}
12 changes: 6 additions & 6 deletions packages/web-components/fast-foundation/docs/api-report.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -476,10 +476,10 @@ export class Combobox extends FormAssociatedCombobox {
placeholder: string;
// @internal
protected placeholderChanged(): void;
position: SelectPosition;
positionAttribute: SelectPosition;
position?: SelectPosition;
positionAttribute?: SelectPosition;
// (undocumented)
protected positionChanged(): void;
protected positionChanged(prev: SelectPosition | undefined, next: SelectPosition | undefined): void;
// @internal
selectedIndexChanged(prev: number | undefined, next: number): void;
// @internal
Expand Down Expand Up @@ -2188,10 +2188,10 @@ export class Select extends FormAssociatedSelect {
open: boolean;
// @internal
protected openChanged(prev: boolean | undefined, next: boolean): void;
position: SelectPosition;
positionAttribute: SelectPosition;
position?: SelectPosition;
positionAttribute?: SelectPosition;
// (undocumented)
protected positionChanged(): void;
protected positionChanged(prev: SelectPosition | undefined, next: SelectPosition | undefined): void;
// @internal
selectedIndexChanged(prev: number | undefined, next: number): void;
// @internal @override
Expand Down
22 changes: 11 additions & 11 deletions packages/web-components/fast-foundation/src/combobox/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,8 @@ See [listbox-option](/docs/components/listbox-option) for more information.
| `open` | public | `boolean` | `false` | The open attribute. | |
| `options` | public | `ListboxOption[]` | | The list of options. | Listbox |
| `placeholder` | public | `string` | | Sets the placeholder value of the element, generally used to provide a hint to the user. | |
| `positionAttribute` | public | `SelectPosition` | | The placement for the listbox when the combobox is open. | |
| `position` | public | `SelectPosition` | | The current state of the calculated position of the listbox. | |
| `positionAttribute` | public | `SelectPosition or undefined` | | The placement for the listbox when the combobox is open. | |
| `position` | public | `SelectPosition or undefined` | | The current state of the calculated position of the listbox. | |
| `value` | public | | | The value property. | |
| `proxy` | | | | | FormAssociatedCombobox |
| `length` | public | `number` | | The number of options. | Listbox |
Expand All @@ -136,15 +136,15 @@ See [listbox-option](/docs/components/listbox-option) for more information.

#### Methods

| Name | Privacy | Description | Parameters | Return | Inherited From |
| -------------------- | --------- | -------------------------------------------------------------------------- | ---------- | ------ | ----------------- |
| `positionChanged` | protected | | | | |
| `filterOptions` | public | Filter available options by text value. | | `void` | |
| `setPositioning` | public | Calculate and apply listbox positioning based on available viewport space. | `force` | `void` | |
| `selectFirstOption` | public | Moves focus to the first selectable option. | | `void` | Listbox |
| `setSelectedOptions` | public | Sets an option as selected and gives it focus. | | | Listbox |
| `templateChanged` | protected | | | `void` | FoundationElement |
| `stylesChanged` | protected | | | `void` | FoundationElement |
| Name | Privacy | Description | Parameters | Return | Inherited From |
| -------------------- | --------- | -------------------------------------------------------------------------- | ---------------------------------------------------------------------- | ------ | ----------------- |
| `positionChanged` | protected | | `prev: SelectPosition or undefined, next: SelectPosition or undefined` | `void` | |
| `filterOptions` | public | Filter available options by text value. | | `void` | |
| `setPositioning` | public | Calculate and apply listbox positioning based on available viewport space. | `force` | `void` | |
| `selectFirstOption` | public | Moves focus to the first selectable option. | | `void` | Listbox |
| `setSelectedOptions` | public | Sets an option as selected and gives it focus. | | | Listbox |
| `templateChanged` | protected | | | `void` | FoundationElement |
| `stylesChanged` | protected | | | `void` | FoundationElement |

#### Events

Expand Down
17 changes: 10 additions & 7 deletions packages/web-components/fast-foundation/src/combobox/combobox.ts
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { attr, DOM, Observable, observable } from "@microsoft/fast-element";
import type { SyntheticViewTemplate } from "@microsoft/fast-element";
import { attr, DOM, Observable, observable } from "@microsoft/fast-element";
import { limit, uniqueId } from "@microsoft/fast-web-utilities";
import type { FoundationElementDefinition } from "../foundation-element/foundation-element.js";
import { DelegatesARIAListbox } from "../listbox/listbox.js";
import type { ListboxOption } from "../listbox-option/listbox-option.js";
import { StartEnd } from "../patterns/start-end.js";
import { DelegatesARIAListbox } from "../listbox/listbox.js";
import type { StartEndOptions } from "../patterns/start-end.js";
import { StartEnd } from "../patterns/start-end.js";
import { SelectPosition } from "../select/select.options.js";
import { applyMixins } from "../utilities/apply-mixins.js";
import { FormAssociatedCombobox } from "./combobox.form-associated.js";
Expand Down Expand Up @@ -209,17 +209,20 @@ export class Combobox extends FormAssociatedCombobox {
* @public
*/
@attr({ attribute: "position" })
public positionAttribute: SelectPosition;
public positionAttribute?: SelectPosition;

/**
* The current state of the calculated position of the listbox.
*
* @public
*/
@observable
public position: SelectPosition = SelectPosition.below;
protected positionChanged() {
this.positionAttribute = this.position;
public position?: SelectPosition;
protected positionChanged(
prev: SelectPosition | undefined,
next: SelectPosition | undefined
): void {
this.positionAttribute = next;
this.setPositioning();
}

Expand Down
22 changes: 11 additions & 11 deletions packages/web-components/fast-foundation/src/select/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ See [listbox-option](/docs/components/listbox-option) for more information.
| ------------------- | --------- | ------------------------------------- | ------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------- |
| `open` | public | `boolean` | `false` | The open attribute. | |
| `value` | public | | | The value property. | |
| `positionAttribute` | public | `SelectPosition` | | Reflects the placement for the listbox when the select is open. | |
| `position` | public | `SelectPosition` | | Holds the current state for the calculated position of the listbox. | |
| `positionAttribute` | public | `SelectPosition or undefined` | | Reflects the placement for the listbox when the select is open. | |
| `position` | public | `SelectPosition or undefined` | | Holds the current state for the calculated position of the listbox. | |
| `displayValue` | public | `string` | | The value displayed on the button. | |
| `proxy` | | | | | FormAssociatedSelect |
| `multiple` | public | `boolean` | | Indicates if the listbox is in multi-selection mode. | ListboxElement |
Expand All @@ -126,15 +126,15 @@ See [listbox-option](/docs/components/listbox-option) for more information.

#### Methods

| Name | Privacy | Description | Parameters | Return | Inherited From |
| -------------------- | --------- | -------------------------------------------------------------------------- | ------------------------------------------- | ------ | ----------------- |
| `positionChanged` | protected | | | | |
| `setPositioning` | public | Calculate and apply listbox positioning based on available viewport space. | | `void` | |
| `multipleChanged` | public | Sets the multiple property on the proxy element. | `prev: boolean or undefined, next: boolean` | | |
| `setSelectedOptions` | public | Sets an option as selected and gives it focus. | | | Listbox |
| `selectFirstOption` | public | Moves focus to the first selectable option. | | `void` | Listbox |
| `templateChanged` | protected | | | `void` | FoundationElement |
| `stylesChanged` | protected | | | `void` | FoundationElement |
| Name | Privacy | Description | Parameters | Return | Inherited From |
| -------------------- | --------- | -------------------------------------------------------------------------- | ---------------------------------------------------------------------- | ------ | ----------------- |
| `positionChanged` | protected | | `prev: SelectPosition or undefined, next: SelectPosition or undefined` | `void` | |
| `setPositioning` | public | Calculate and apply listbox positioning based on available viewport space. | | `void` | |
| `multipleChanged` | public | Sets the multiple property on the proxy element. | `prev: boolean or undefined, next: boolean` | | |
| `setSelectedOptions` | public | Sets an option as selected and gives it focus. | | | Listbox |
| `selectFirstOption` | public | Moves focus to the first selectable option. | | `void` | Listbox |
| `templateChanged` | protected | | | `void` | FoundationElement |
| `stylesChanged` | protected | | | `void` | FoundationElement |

#### Events

Expand Down
11 changes: 7 additions & 4 deletions packages/web-components/fast-foundation/src/select/select.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ export class Select extends FormAssociatedSelect {
* @public
*/
@attr({ attribute: "position" })
public positionAttribute: SelectPosition;
public positionAttribute?: SelectPosition;

/**
* Indicates the initial state of the position attribute.
Expand All @@ -210,9 +210,12 @@ export class Select extends FormAssociatedSelect {
* @public
*/
@observable
public position: SelectPosition = SelectPosition.below;
protected positionChanged() {
this.positionAttribute = this.position;
public position?: SelectPosition;
protected positionChanged(
prev: SelectPosition | undefined,
next: SelectPosition | undefined
): void {
this.positionAttribute = next;
this.setPositioning();
}

Expand Down

0 comments on commit 10169e7

Please sign in to comment.