Skip to content

Commit

Permalink
fix(select): updating select based on feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
Nikki Massaro Kauffman committed Oct 23, 2023
1 parent e815f68 commit 197117e
Show file tree
Hide file tree
Showing 10 changed files with 175 additions and 162 deletions.
36 changes: 15 additions & 21 deletions core/pfe-core/controllers/listbox-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ export class ListboxController<

/** event listeners for host element */
#listeners = {
'click': this.#onOptionClick.bind(this),
'focus': this.#onOptionFocus.bind(this),
'keydown': this.#onOptionKeydown.bind(this),
'keyup': this.#onOptionKeyup.bind(this),
'optionfocus': this.#onOptionFocus.bind(this),
'click': this.#onOptionClick.bind(this),
};

/**
Expand Down Expand Up @@ -224,18 +224,7 @@ export class ListboxController<
* adds event listeners to host
*/
hostConnected() {
for (const [event, listener] of Object.entries(this.#listeners)) {
this.host?.addEventListener(event, listener as (event: Event | null) => void);
}
}

/**
* removes event listeners from host
*/
hostDisconnected() {
for (const [event, listener] of Object.entries(this.#listeners)) {
this.host?.removeEventListener(event, listener as (event: Event | null) => void);
}
this.#internals.role = 'listbox';
}

/**
Expand Down Expand Up @@ -264,12 +253,12 @@ export class ListboxController<
}
this.options.forEach(option => {
if (matchedOptions.includes(option)) {
option.removeAttribute('hidden-by-filter');
option.removeAttribute('filtered');
} else {
if (document.activeElement === option) {
this.#updateFocus = true;
}
option.setAttribute('hidden-by-filter', 'hidden-by-filter');
option.setAttribute('filtered', 'filtered');
}
});
return matchedOptions;
Expand Down Expand Up @@ -492,11 +481,16 @@ export class ListboxController<
* @param oldOptions {ListboxOptionElement[]}
*/
#optionsChanged(oldOptions: ListboxOptionElement[]) {
const setSize = oldOptions.length;
if (setSize !== this.#options.length || !oldOptions.every((element, index) => element === this.#options[index])) {
oldOptions.forEach((option, posInSet) => {
option.setSize = setSize;
option.posInSet = posInSet;
const setSize = this.#options.length;
if (setSize !== oldOptions.length || !oldOptions.every((element, index) => element === this.#options[index])) {
this.#options.forEach((option, posInSet) => {
if (!oldOptions.includes(option)) {
option.setSize = setSize;
option.posInSet = posInSet;
for (const [event, listener] of Object.entries(this.#listeners)) {
option?.addEventListener(event, listener as (event: Event | null) => void);
}
}
});
this.#tabindex.initItems(this.visibleOptions);
}
Expand Down
6 changes: 0 additions & 6 deletions core/pfe-core/controllers/toggle-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -323,9 +323,6 @@ export class ToggleController implements ReactiveController {
for (const [event, listener] of Object.entries(this.#triggerListeners)) {
triggerElement?.addEventListener(event, listener as (event: KeyboardEvent | MouseEvent | Event | null) => void);
}
for (const [event, listener] of Object.entries(this.#hostListeners)) {
triggerElement?.addEventListener(event, listener as (event: KeyboardEvent | MouseEvent | Event | null) => void);
}
triggerElement?.setAttribute('aria-haspopup', this.#popupType);
triggerElement?.setAttribute('aria-controls', this.#popupElement?.id || '');
triggerElement?.setAttribute('aria-expanded', this.expanded ? 'true' : 'false');
Expand All @@ -342,9 +339,6 @@ export class ToggleController implements ReactiveController {
for (const [event, listener] of Object.entries(this.#triggerListeners)) {
triggerElement?.removeEventListener(event, listener as (event: KeyboardEvent | MouseEvent | Event | null) => void);
}
for (const [event, listener] of Object.entries(this.#hostListeners)) {
triggerElement?.removeEventListener(event, listener as (event: KeyboardEvent | MouseEvent | Event | null) => void);
}
triggerElement?.removeAttribute('aria-haspopup');
triggerElement?.removeAttribute('aria-controls');
triggerElement?.removeAttribute('aria-expanded');
Expand Down
6 changes: 3 additions & 3 deletions elements/pf-select/demo/multiple-selections.html
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<kbd>Shift</kbd> will toggling off multiple items.
<kbd>Ctrl+A</kbd> will toggle selection on all items.
</p>
<pf-select multi-selectable>
<pf-select multi>
<pf-select-option value="Blue">Blue</pf-select-option>
<pf-select-option value="Green">Green</pf-select-option>
<pf-select-option value="Magenta">Magenta</pf-select-option>
Expand Down Expand Up @@ -41,7 +41,7 @@
<p>
Display as badge
</p>
<pf-select multi-selectable selected-items-display="badge">
<pf-select multi selected-items-display="badge">
<pf-select-option value="Blue">Blue</pf-select-option>
<pf-select-option value="Green">Green</pf-select-option>
<pf-select-option value="Magenta">Magenta</pf-select-option>
Expand All @@ -57,7 +57,7 @@
<p>
Display as chips
</p>
<pf-select multi-selectable selected-items-display="chips" enable-flip>
<pf-select multi selected-items-display="chips" enable-flip>
<pf-select-option value="Blue">Blue</pf-select-option>
<pf-select-option value="Green">Green</pf-select-option>
<pf-select-option value="Magenta">Magenta</pf-select-option>
Expand Down
4 changes: 2 additions & 2 deletions elements/pf-select/demo/typeahead.html
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ <h2>Typeahead example</h2>
<h2>Typeahead example with chips</h2>
<label>
Pick a color:
<pf-select typeahead multi-selectable selected-items-display="chips">
<pf-select typeahead multi selected-items-display="chips">
<pf-select-option value="Blue">Blue</pf-select-option>
<pf-select-option value="Green">Green</pf-select-option>
<pf-select-option value="Magenta">Magenta</pf-select-option>
Expand Down Expand Up @@ -59,7 +59,7 @@ <h2>Typeahead example with create option</h2>
<h2>Typeahead, multiple</h2>
<label>
Pick a color:
<pf-select id="pfselect" typeahead multi-selectable>
<pf-select id="pfselect" typeahead multi>
<pf-select-option value="Blue">Blue</pf-select-option>
<pf-select-option value="Green">Green</pf-select-option>
<pf-select-option value="Magenta">Magenta</pf-select-option>
Expand Down
10 changes: 5 additions & 5 deletions elements/pf-select/docs/pf-select.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ Below are option variants:
</p>

{% htmlexample %}
<pf-select multi-selectable>
<pf-select multi>
<pf-select-option value="Blue">Blue</pf-select-option>
<pf-select-option value="Green">Green</pf-select-option>
<pf-select-option value="Magenta">Magenta</pf-select-option>
Expand Down Expand Up @@ -186,12 +186,12 @@ Below are option variants:
</label>
{% endhtmlexample %}

#### Typeahead, multi-selectable with chips
#### Typeahead, multi with chips

{% htmlexample %}
<label>
Pick a color:
<pf-select typeahead multi-selectable selected-items-display="chips">
<pf-select typeahead multi selected-items-display="chips">
<pf-select-option value="Blue">Blue</pf-select-option>
<pf-select-option value="Green">Green</pf-select-option>
<pf-select-option value="Magenta">Magenta</pf-select-option>
Expand All @@ -204,12 +204,12 @@ Below are option variants:
</label>
{% endhtmlexample %}

#### Typeahead, multi-selectable with create option
#### Typeahead, multi with create option

{% htmlexample %}
<label>
Pick a color:
<pf-select id="pfselect" typeahead multi-selectable create-option-text="Create option">
<pf-select id="pfselect" typeahead multi create-option-text="Create option">
<pf-select-option value="Blue">Blue</pf-select-option>
<pf-select-option value="Green">Green</pf-select-option>
<pf-select-option value="Magenta">Magenta</pf-select-option>
Expand Down
33 changes: 24 additions & 9 deletions elements/pf-select/pf-select-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,19 @@ import { ListboxController, type ListboxOptionElement } from '@patternfly/pfe-co
import { PfSelectGroup } from './pf-select-group.js';
import { PfSelectOption } from './pf-select-option.js';


import styles from './pf-select-list.css';

/**
* event when slotted options are updated
* @first refresh
*/
export class PfSelectListRefreshEvent extends Event {
constructor(public originalEvent: Event) {
super('refresh', { bubbles: true, composed: true });
}
}

/**
* Listbox for select options.
*
Expand Down Expand Up @@ -39,7 +51,7 @@ export class PfSelectList extends LitElement {
/**
* whether multiple items can be selected
*/
@property({ reflect: true, attribute: 'multi-selectable', type: Boolean }) multiSelectable = false;
@property({ reflect: true, attribute: 'multi', type: Boolean }) multi = false;

@queryAssignedElements({ flatten: true }) private _slottedElements?: Array<HTMLElement>;

Expand Down Expand Up @@ -115,10 +127,11 @@ export class PfSelectList extends LitElement {
this.#listboxController = new ListboxController(this, {
caseSensitive: this.caseSensitive,
matchAnywhere: this.matchAnywhere,
multi: this.multiSelectable,
multi: this.multi,
orientation: 'vertical'
});
this.#listboxController.options = this.options as ListboxOptionElement[];
const options: unknown[] = this.options || [];
this.#listboxController.options = options as ListboxOptionElement[];
super.firstUpdated(changed);
}

Expand All @@ -133,8 +146,8 @@ export class PfSelectList extends LitElement {
if (changed.has('matchAnywhere')) {
this.#listboxController.matchAnywhere = this.matchAnywhere;
}
if (changed.has('multiSelectable')) {
this.#listboxController.multi = this.multiSelectable;
if (changed.has('multi')) {
this.#listboxController.multi = this.multi;
}
}
}
Expand Down Expand Up @@ -162,13 +175,15 @@ export class PfSelectList extends LitElement {
}

/**
* @fires listboxoptions to indicate slotted listbox options ahve changed
* handles slot change to indicate slotted listbox options have changed
* @param event {Event}
*/
#onSlotchange() {
#onSlotchange(event: Event) {
if (this.#listboxController) {
this.#listboxController.options = this.options as ListboxOptionElement[];
const options: unknown[] = this.options || [];
this.#listboxController.options = options as ListboxOptionElement[];
}
this.dispatchEvent(new Event('listboxoptions', { bubbles: true }));
this.dispatchEvent(new PfSelectListRefreshEvent(event));
}
}

Expand Down
1 change: 0 additions & 1 deletion elements/pf-select/pf-select-option.css
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
}

:host([hidden]),
:host([hidden-by-filter]),
*[hidden] {
display: none !important;
}
Expand Down
Loading

0 comments on commit 197117e

Please sign in to comment.