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.

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

feat(handle): Add blurUnselectDisabled property to disable unselecting handle on blur. #8483

Merged
merged 8 commits into from
Dec 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions packages/calcite-components/src/components.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1769,6 +1769,10 @@ export namespace Components {
}
interface CalciteHandle {
"activated": boolean;
/**
* When `true`, disables unselecting the component when blurred.
*/
"blurUnselectDisabled": boolean;
/**
* When `true`, interaction is prevented and the component is displayed with lower opacity.
*/
Expand Down Expand Up @@ -9063,6 +9067,10 @@ declare namespace LocalJSX {
}
interface CalciteHandle {
"activated"?: boolean;
/**
* When `true`, disables unselecting the component when blurred.
*/
"blurUnselectDisabled"?: boolean;
/**
* When `true`, interaction is prevented and the component is displayed with lower opacity.
*/
Expand Down
45 changes: 45 additions & 0 deletions packages/calcite-components/src/components/handle/handle.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,51 @@ describe("calcite-handle", () => {
expect(await handle.getProperty("activated")).toBe(true);
});

it("sets activated to false when blurred", async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this fix any behavior in list or another consuming component? If so, is it worth adding a check there? Disregard if not the case.

const page = await newE2EPage();
await page.setContent("<calcite-handle></calcite-handle>");

const handle = await page.find("calcite-handle");
const button = await page.find(`calcite-handle >>> .${CSS.handle}`);

expect(await handle.getProperty("activated")).toBe(false);

await button.focus();

await page.keyboard.press(" ");

await page.waitForChanges();

expect(await handle.getProperty("activated")).toBe(true);

await page.$eval("calcite-handle", (handle: HTMLCalciteHandleElement) => handle.blur());

expect(await handle.getProperty("activated")).toBe(false);
});

it("does not set activated to false when blurUnselectDisabled and blurred", async () => {
const page = await newE2EPage();
await page.setContent("<calcite-handle blur-unselect-disabled></calcite-handle>");

const handle = await page.find("calcite-handle");
const button = await page.find(`calcite-handle >>> .${CSS.handle}`);

expect(await handle.getProperty("blurUnselectDisabled")).toBe(true);
expect(await handle.getProperty("activated")).toBe(false);

await button.focus();

await page.keyboard.press(" ");

await page.waitForChanges();

expect(await handle.getProperty("activated")).toBe(true);

await page.$eval("calcite-handle", (handle: HTMLCalciteHandleElement) => handle.blur());

expect(await handle.getProperty("activated")).toBe(true);
});

it("fires calciteHandleNudge event when focused and up or down key is pressed", async () => {
const page = await newE2EPage();
await page.setContent("<calcite-handle></calcite-handle>");
Expand Down
9 changes: 8 additions & 1 deletion packages/calcite-components/src/components/handle/handle.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,13 @@ export class Handle implements LoadableComponent, T9nComponent, InteractiveCompo
*/
@Prop() label: string;

/**
* When `true`, disables unselecting the component when blurred.
*
* @internal
*/
@Prop() blurUnselectDisabled = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a way to tie this name to activated, or would that not be accurate? Just a nitpick as its an internal prop, but its a bit confusing if unfamiliar with the component.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was thinking that but I think the next step is going to be renaming active to selected and making it public.


/**
* Use this property to override individual strings used by the component.
*/
Expand Down Expand Up @@ -251,7 +258,7 @@ export class Handle implements LoadableComponent, T9nComponent, InteractiveCompo
};

handleBlur = (): void => {
if (this.disabled) {
if (this.blurUnselectDisabled || this.disabled) {
return;
}

Expand Down
5 changes: 2 additions & 3 deletions packages/calcite-components/src/components/list/list.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -954,6 +954,7 @@ export class List
}

this.disconnectObserver();
handle.blurUnselectDisabled = true;

const referenceEl =
(direction === "up" && newIndex !== lastIndex) || (direction === "down" && newIndex === 0)
Expand All @@ -973,8 +974,6 @@ export class List
oldIndex,
});

handle.setFocus().then(() => {
handle.activated = true;
});
handle.setFocus().then(() => (handle.blurUnselectDisabled = false));
}
}
Loading