Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(dropdown): open dropdown on ArrowDown & ArrowUp keys #10264

Merged
merged 10 commits into from
Sep 25, 2024
Merged

fix(dropdown): open dropdown on ArrowDown & ArrowUp keys #10264

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
d3370e1
fix(dropdown): open dropdown on ArrowDown & ArrowUp keys
anveshmekala Sep 10, 2024
8940e89
refactor
anveshmekala Sep 10, 2024
95a46f1
cleanup
anveshmekala Sep 10, 2024
520fa62
feedback changes & refactor
anveshmekala Sep 11, 2024
6150a6d
arrowkeys wont close dropdown
anveshmekala Sep 11, 2024
a45b1dc
Merge branch 'dev' into anveshmekala/8205-fix-dropdown-arrow-keys
anveshmekala Sep 11, 2024
a5c193c
revert & add eventListeners
anveshmekala Sep 24, 2024
de0c3eb
avoid closing dropdown on arrow keys
anveshmekala Sep 25, 2024
6b08f1d
add delay for open story
anveshmekala Sep 25, 2024
59836d4
revert delay for open story
anveshmekala Sep 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 27 additions & 38 deletions packages/calcite-components/src/components/dropdown/dropdown.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ export class Dropdown
<div
class="calcite-trigger-container"
id={`${guid}-menubutton`}
onClick={() => this.openCalciteDropdown()}
onClick={this.clickHandler}
onKeyDown={this.keyDownHandler}
ref={this.setReferenceEl}
>
Expand Down Expand Up @@ -354,7 +354,7 @@ export class Dropdown
return;
}

this.openCalciteDropdown();
this.clickHandler();
}

@Listen("pointerleave")
Expand Down Expand Up @@ -445,6 +445,8 @@ export class Dropdown

guid = `calcite-dropdown-${guid()}`;

private focusLastDropdownItem = false;

//--------------------------------------------------------------------------
//
// Private Methods
Expand Down Expand Up @@ -541,6 +543,7 @@ export class Dropdown

onOpen(): void {
this.calciteDropdownOpen.emit();
this.focusOnFirstActiveOrDefaultItem();
}

onBeforeClose(): void {
Expand Down Expand Up @@ -573,26 +576,24 @@ export class Dropdown
return;
}

if (this.open) {
if (key === "Escape") {
this.closeCalciteDropdown();
event.preventDefault();
return;
} else if (event.shiftKey && key === "Tab") {
this.closeCalciteDropdown();
event.preventDefault();
return;
}
if (key === "Escape") {
this.closeCalciteDropdown();
event.preventDefault();
return;
}

if (isActivationKey(key)) {
this.openCalciteDropdown();
event.preventDefault();
} else if (key === "Escape") {
if (this.open && event.shiftKey && key === "Tab") {
this.closeCalciteDropdown();
event.preventDefault();
return;
}

if (isActivationKey(key)) {
this.clickHandler();
event.preventDefault();
} else if (key === "ArrowDown" || key === "ArrowUp") {
this.openCalciteDropdown(key === "ArrowUp");
this.focusLastDropdownItem = key === "ArrowUp";
this.clickHandler();
}
};

Expand Down Expand Up @@ -636,35 +637,23 @@ export class Dropdown
}
}

private focusOnFirstActiveOrDefaultItem = (focusLastItem: boolean): void => {
private focusOnFirstActiveOrDefaultItem = (): void => {
const selectedItem = this.getTraversableItems().find((item) => item.selected);
if (selectedItem) {
this.focusDropdownItemElement(selectedItem);
return;
} else {
this.focusDropdownItemElement(
focusLastItem ? this.items[this.items.length - 1] : this.items[0],
);
}
};
const target: HTMLCalciteDropdownItemElement =
selectedItem ||
(this.focusLastDropdownItem ? this.items[this.items.length - 1] : this.items[0]);

this.focusLastDropdownItem = false;

private focusDropdownItemElement(item: HTMLCalciteDropdownItemElement): void {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

took the liberty and refactored. @jcfranco can i get one more review?

if (!item) {
if (!target) {
return;
}
focusElement(item);
}

private toggleOpenEnd = (focusLastItem: boolean): void => {
this.focusOnFirstActiveOrDefaultItem(focusLastItem);
this.el.removeEventListener("calciteDropdownOpen", () => this.toggleOpenEnd(focusLastItem));
focusElement(target);
};

private openCalciteDropdown = (focusLastItem = false): void => {
private clickHandler = (): void => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we keep this dedicated click handler and restore the previous openCalciteDropdown (name could be simplified to openDropdown)? That way, other event handlers can call this dedicated open method instead of invoking an event handler for a different event.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think clickHandler( ) isn't required and we can use toggleDropdown (instead of openDropdown) method to handle click.

this.open = !this.open;
if (this.open) {
this.el.addEventListener("calciteDropdownOpen", () => this.toggleOpenEnd(focusLastItem));
}
};

private updateTabIndexOfItems(target: HTMLCalciteDropdownItemElement): void {
Expand Down
Loading