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

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

Merged
merged 4 commits into from
Sep 24, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
05e947c
refactor: simplify reflected attribute accesses
maxpatiiuk Sep 19, 2024
9e64d87
refactor: use correct input-text type
maxpatiiuk Sep 19, 2024
5696e22
refactor(link): don't conflate JSX tag name and string
maxpatiiuk Sep 24, 2024
a73a79d
fix(Heading): remove non-existent dom prop
maxpatiiuk Sep 24, 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
2 changes: 1 addition & 1 deletion packages/calcite-components/src/components/button/button.tsx
View file
Open in desktop
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
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion packages/calcite-components/src/components/functional/Heading.tsx
View file
Open in desktop
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
8 changes: 4 additions & 4 deletions packages/calcite-components/src/components/input-date-picker/input-date-picker.tsx
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions packages/calcite-components/src/components/input-time-picker/input-time-picker.tsx
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
3 changes: 0 additions & 3 deletions packages/calcite-components/src/components/tooltip/tooltip.tsx
View file
Open in desktop
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