Skip to content

Commit

Permalink
refactor(openClose): use the correct property to specify open/close t…
Browse files Browse the repository at this point in the history
…ransition prop (#11286)

**Related Issue:** N/A

## Summary

Updates open/close components to use the correct transition property.

### Notes

* this was working as expected because the property wasn't being used
correctly
* makes `openProp` optional and defaults to `open` when not specified
* updates types to ensure CSS props are specified by `transitionProp`
* renames `openTransitionProp` to `openProp` to avoid confusion with CSS
transitions

@Elijbet Thanks for spotting this one!
  • Loading branch information
jcfranco authored Jan 15, 2025
1 parent 2bb1ef5 commit 8c09c74
Show file tree
Hide file tree
Showing 14 changed files with 35 additions and 29 deletions.
2 changes: 1 addition & 1 deletion packages/calcite-components/src/components/alert/alert.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export class Alert extends LitElement implements OpenCloseComponent, LoadableCom

private lastMouseOverBegin: number;

openTransitionProp = "opacity";
transitionProp = "opacity" as const;

private totalHoverTime = 0;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ export class Autocomplete
*/
messages = useT9n<typeof T9nStrings>();

openTransitionProp = "opacity";
transitionProp = "opacity" as const;

referenceEl: Input["el"];

Expand Down
2 changes: 1 addition & 1 deletion packages/calcite-components/src/components/block/block.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export class Block

// #region Private Properties

openTransitionProp = "margin-top";
transitionProp = "margin-top" as const;

transitionEl: HTMLElement;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ export class Combobox
this.setFocus();
};

openTransitionProp = "opacity";
transitionProp = "opacity" as const;

placement: LogicalPlacement = defaultMenuPlacement;

Expand Down
4 changes: 3 additions & 1 deletion packages/calcite-components/src/components/dialog/dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,9 @@ export class Dialog
) /* TODO: [MIGRATION] If possible, refactor to use on* JSX prop or this.listen()/this.listenOn() utils - they clean up event listeners automatically, thus prevent memory leaks */;
};

openTransitionProp = "opacity";
openProp = "opened";

transitionProp = "opacity" as const;

private panelEl = createRef<Panel["el"]>();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export class Dropdown
) /* TODO: [MIGRATION] If possible, refactor to use on* JSX prop or this.listen()/this.listenOn() utils - they clean up event listeners automatically, thus prevent memory leaks */;
};

openTransitionProp = "opacity";
transitionProp = "opacity" as const;

referenceEl: HTMLDivElement;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ export class InputDatePicker

labelEl: Label["el"];

openTransitionProp = "opacity";
transitionProp = "opacity" as const;

private placeholderTextId = `calcite-input-date-picker-placeholder-${guid()}`;

Expand Down
4 changes: 3 additions & 1 deletion packages/calcite-components/src/components/modal/modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,9 @@ export class Modal
) /* TODO: [MIGRATION] If possible, refactor to use on* JSX prop or this.listen()/this.listenOn() utils - they clean up event listeners automatically, thus prevent memory leaks */;
};

openTransitionProp = "opacity";
openProp = "opened";

transitionProp = "opacity" as const;

private titleId: string;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export class Notice extends LitElement implements LoadableComponent, OpenCloseCo
/** The close button element. */
private closeButton = createRef<HTMLButtonElement>();

openTransitionProp = "opacity";
transitionProp = "opacity" as const;

/** The computed icon to render. */
private requestedIcon?: IconNameOrString;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ export class Popover
this.updateFocusTrapElements(),
);

openTransitionProp = "opacity";
transitionProp = "opacity" as const;

transitionEl: HTMLDivElement;

Expand Down
4 changes: 3 additions & 1 deletion packages/calcite-components/src/components/sheet/sheet.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,9 @@ export class Sheet
) /* TODO: [MIGRATION] If possible, refactor to use on* JSX prop or this.listen()/this.listenOn() utils - they clean up event listeners automatically, thus prevent memory leaks */;
};

openTransitionProp = "opacity";
openProp = "opened";

transitionProp = "opacity" as const;

private resizeHandleEl: HTMLDivElement;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export class Tooltip extends LitElement implements FloatingUIComponent, OpenClos

private guid = `calcite-tooltip-${guid()}`;

openTransitionProp = "opacity";
transitionProp = "opacity" as const;

transitionEl: HTMLDivElement;

Expand Down
11 changes: 6 additions & 5 deletions packages/calcite-components/src/utils/openCloseComponent.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ describe("openCloseComponent", () => {
const fakeOpenCloseComponent = {
el: document.createElement("div"),
open: true,
openTransitionProp: "opacity",
transitionProp: "opacity" as const,
openTransitionProp: "open",
transitionEl,
onBeforeOpen: vi.fn(() => emittedEvents.push("beforeOpen")),
onOpen: vi.fn(() => emittedEvents.push("open")),
Expand All @@ -50,21 +51,21 @@ describe("openCloseComponent", () => {
onToggleOpenCloseComponent(fakeOpenCloseComponent);
expect(emittedEvents).toEqual([]);

dispatchTransitionEvent(transitionEl, "transitionstart", fakeOpenCloseComponent.openTransitionProp);
dispatchTransitionEvent(transitionEl, "transitionstart", fakeOpenCloseComponent.transitionProp);
expect(emittedEvents).toEqual(["beforeOpen"]);

dispatchTransitionEvent(transitionEl, "transitionend", fakeOpenCloseComponent.openTransitionProp);
dispatchTransitionEvent(transitionEl, "transitionend", fakeOpenCloseComponent.transitionProp);
expect(emittedEvents).toEqual(["beforeOpen", "open"]);

fakeOpenCloseComponent.open = false;

onToggleOpenCloseComponent(fakeOpenCloseComponent);
expect(emittedEvents).toEqual(["beforeOpen", "open"]);

dispatchTransitionEvent(transitionEl, "transitionstart", fakeOpenCloseComponent.openTransitionProp);
dispatchTransitionEvent(transitionEl, "transitionstart", fakeOpenCloseComponent.transitionProp);
expect(emittedEvents).toEqual(["beforeOpen", "open", "beforeClose"]);

dispatchTransitionEvent(transitionEl, "transitionend", fakeOpenCloseComponent.openTransitionProp);
dispatchTransitionEvent(transitionEl, "transitionend", fakeOpenCloseComponent.transitionProp);
expect(emittedEvents).toEqual(["beforeOpen", "open", "beforeClose", "close"]);
});
});
Expand Down
23 changes: 11 additions & 12 deletions packages/calcite-components/src/utils/openCloseComponent.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// @ts-strict-ignore
import { KebabCase } from "type-fest";
import { whenTransitionDone } from "./dom";

/**
Expand All @@ -9,17 +10,15 @@ export interface OpenCloseComponent {
/** The host element. */
readonly el: HTMLElement;

/** When true, the component opens. */
open?: boolean;
/**
* Specifies property on which active transition is watched for.
*
* This should be used if the component uses a property other than `open` to trigger a transition.
*/
openProp?: string;

/** When true, the component is open. */
opened?: boolean;

/** Specifies the name of transitionProp. */
transitionProp?: string;

/** Specifies property on which active transition is watched for. */
openTransitionProp: string;
/** Specifies the name of CSS transition property. */
transitionProp?: KebabCase<Extract<keyof CSSStyleDeclaration, string>>;

/** Specifies element that the transition is allowed to emit on. */
transitionEl: HTMLElement;
Expand All @@ -38,7 +37,7 @@ export interface OpenCloseComponent {
}

function isOpen(component: OpenCloseComponent): boolean {
return "opened" in component ? component.opened : component.open;
return component[component.openProp || "open"];
}

/**
Expand Down Expand Up @@ -67,7 +66,7 @@ export function onToggleOpenCloseComponent(component: OpenCloseComponent): void

whenTransitionDone(
component.transitionEl,
component.openTransitionProp,
component.transitionProp,
() => {
if (isOpen(component)) {
component.onBeforeOpen();
Expand Down

0 comments on commit 8c09c74

Please sign in to comment.