Skip to content

Commit

Permalink
fix: wait for fonts ready before positioning overlays
Browse files Browse the repository at this point in the history
  • Loading branch information
@Westbrook
Westbrook committed Feb 24, 2021
1 parent 1e54852 commit cb8026a
Showing 1 changed file with 9 additions and 0 deletions.
9 changes: 9 additions & 0 deletions packages/overlay/src/ActiveOverlay.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,14 @@ export interface PositionResult {
positionTop: number;
}

declare global {

This comment has been minimized.

Sign in to view

Copy link
@joekukish

joekukish Mar 12, 2021

Collaborator

@Westbrook You can use @types/css-font-loading-module instead.

This comment has been minimized.

Sign in to view

Copy link
@Westbrook

Westbrook Mar 12, 2021

Author Contributor

Thanks for the info. I'm still kind of finding my way through the dark on the TS stuff. Have you run into any issue with the current set up?

This comment has been minimized.

Sign in to view

Copy link
@joekukish

joekukish Mar 12, 2021 •
edited
Loading

Collaborator

@Westbrook yes, that's how I discovered it. here fonts is defined as optional and css-font-loading-module does not so I was getting an error that both declarations are not compatible. Sadly css-font-loading-module is not complete either (add, check, delete are missing from the type declaration), which is what I need.

What we could also do, is that you declare it like this

declare interface FontFaceSet {
    readonly status: FontFaceSetStatus;
    readonly ready: Promise<FontFaceSet>;
    add(font: FontFace): void;
    check(font: string, text?: string): boolean; // throws exception
    load(font: string, text?: string): Promise<FontFace[]>;
    delete(font: FontFace): void;
    clear(): void;
}

declare interface Document {
    fonts?: FontFaceSet;
}

So we have a more complete FontFaceSet API.

This comment has been minimized.

Sign in to view

Copy link
@Westbrook

Westbrook Mar 12, 2021

Author Contributor

If you're not getting enough from @types/css-font-loading-module in a way that you'd need something custom anyways, I'd love to see a PR with this addition. We've not done a lot of organized type delivery in the project to day, but we do use this simplified declaration in a couple of places, so if you'd prefer to move us over to @types/css-font-loading-module and then extend as needed in your project, that makes sense, too.
interface Document {
fonts?: {
ready: Promise<void>;
};
}
}

type OverlayStateType =
| 'idle'
| 'active'
Expand Down Expand Up @@ -392,6 +400,7 @@ export class ActiveOverlay extends SpectrumElement {

public async updateOverlayPosition(): Promise<void> {
if (this.popper) {
await (document.fonts ? document.fonts.ready : Promise.resolve());
await this.popper.update();
}
}
Expand Down

0 comments on commit cb8026a

Please sign in to comment.