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

refactor(components): reduce post-migration TypeScript errors #10356

Merged
merged 4 commits into from
Sep 24, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ export class Button
<InteractiveContainer disabled={this.disabled}>
<Tag
aria-disabled={childElType === "a" ? toAriaBoolean(this.disabled || this.loading) : null}
aria-expanded={this.el.getAttribute("aria-expanded")}
aria-expanded={this.el.ariaExpanded}
aria-label={!this.loading ? getLabelText(this) : this.messages.loading}
aria-live="polite"
class={{
Expand Down
7 changes: 0 additions & 7 deletions packages/calcite-components/src/components/dialog/dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,6 @@ export class Dialog
[CSS.containerOpen]: opened,
[CSS.containerEmbedded]: this.embedded,
}}
ref={this.setContainerEl}
>
{this.modal ? (
<calcite-scrim class={CSS.scrim} onClick={this.handleOutsideClose} />
Expand Down Expand Up @@ -364,8 +363,6 @@ export class Dialog

transitionEl: HTMLDivElement;

containerEl: HTMLDivElement;

focusTrap: FocusTrap;

private resizePosition: DialogResizePosition = { ...initialResizePosition };
Expand Down Expand Up @@ -764,10 +761,6 @@ export class Dialog
}
}

private setContainerEl = (el: HTMLDivElement): void => {
Copy link
Member Author

Choose a reason for hiding this comment

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

Never read, thus removed

this.containerEl = el;
};

private setTransitionEl = (el: HTMLDivElement): void => {
this.transitionEl = el;
this.setupInteractions();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export const Heading: FunctionalComponent<HeadingProps> = (props, children): VNo
delete props.level;

return (
<HeadingTag class={props.class} key={props.key} level={props.level}>
Copy link
Member Author

Choose a reason for hiding this comment

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

level prop doesn't seem to exist in the DOM?
was it used for styling or testing? if not, we should remove it


original discussion in #10352 (comment)

<HeadingTag class={props.class} key={props.key}>
{children}
</HeadingTag>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -774,9 +774,9 @@ export class InputDatePicker

@State() private localeData: DateLocaleData;

private startInput: HTMLCalciteInputElement;
private startInput: HTMLCalciteInputTextElement;
Copy link
Member Author

Choose a reason for hiding this comment

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

This was causing a type error in Lumina - the ref is assigned to calcite-input-text, but the type here was calcite-input.


private endInput: HTMLCalciteInputElement;
private endInput: HTMLCalciteInputTextElement;

private floatingEl: HTMLDivElement;

Expand Down Expand Up @@ -884,11 +884,11 @@ export class InputDatePicker
syncHiddenFormInput("date", this, input);
}

setStartInput = (el: HTMLCalciteInputElement): void => {
setStartInput = (el: HTMLCalciteInputTextElement): void => {
this.startInput = el;
};

setEndInput = (el: HTMLCalciteInputElement): void => {
setEndInput = (el: HTMLCalciteInputTextElement): void => {
this.endInput = el;
};

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

@Element() el: HTMLCalciteInputTimePickerElement;

@State() calciteInputEl: HTMLCalciteInputElement;
@State() calciteInputEl: HTMLCalciteInputTextElement;

defaultValue: InputTimePicker["value"];

Expand Down Expand Up @@ -853,7 +853,7 @@ export class InputTimePicker
this.openHandler();
};

private setInputEl = (el: HTMLCalciteInputElement): void => {
private setInputEl = (el: HTMLCalciteInputTextElement): void => {
this.calciteInputEl = el;
};

Expand Down
12 changes: 8 additions & 4 deletions packages/calcite-components/src/components/link/link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -114,15 +114,19 @@ export class Link implements InteractiveComponent, LoadableComponent {
This works around that issue for now.
*/
download={
Tag === "a" ? (download === true || download === "" ? "" : download || null) : null
childElType === "a"
Copy link
Member Author

Choose a reason for hiding this comment

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

As per line 103 in this file (const Tag = childElType;), Tag and childElType variables have the same value in Stencil - thus this change should have no runtime impact.
However, in Lit, Tag function has a bit different type to comply with Lit's dynamic tag name syntax.

See Lumina's docs on JSX -> Dynamic tag names for more details

? download === true || download === ""
? ""
: download || null
: null
}
href={Tag === "a" && this.href}
href={childElType === "a" && this.href}
onClick={this.childElClickHandler}
ref={this.storeTagRef}
rel={Tag === "a" && this.rel}
rel={childElType === "a" && this.rel}
role={role}
tabIndex={tabIndex}
target={Tag === "a" && this.target}
target={childElType === "a" && this.target}
>
{this.iconStart ? iconStartEl : null}
<slot />
Expand Down
2 changes: 1 addition & 1 deletion packages/calcite-components/src/components/menu/menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ export class CalciteMenu implements LocalizedComponent, T9nComponent, LoadableCo
}

private getEffectiveRole(): string {
return this.el.getAttribute("role") || "menubar";
Copy link
Member Author

Choose a reason for hiding this comment

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

equivalent

return this.el.role || "menubar";
}

// --------------------------------------------------------------------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,6 @@ export class Tooltip implements FloatingUIComponent, OpenCloseComponent {

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

hasLoaded = false;
Copy link
Member Author

Choose a reason for hiding this comment

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

never read


openTransitionProp = "opacity";

transitionEl: HTMLDivElement;
Expand Down Expand Up @@ -175,7 +173,6 @@ export class Tooltip implements FloatingUIComponent, OpenCloseComponent {
if (this.referenceElement && !this.effectiveReferenceElement) {
this.setUpReferenceElement();
}
this.hasLoaded = true;
}

disconnectedCallback(): void {
Expand Down
Loading