Skip to content

i2svg options should be optional - #14512

Open
cooljeffro wants to merge 2 commits into
FortAwesome:masterfrom
cooljeffro:patch-1
Open

cooljeffro wants to merge 2 commits into
FortAwesome:masterfrom
cooljeffro:patch-1

Conversation

@cooljeffro

@cooljeffro cooljeffro commented Jan 17, 2019 •

Copy link
Copy Markdown

In i2svg, when specifying the 'node' param, typescript then requires the 'callback' param. Both 'node' and 'callback' should be optional.

I understand that:

  • I'm submitting this PR for reference only. It shows an example of what I'd like to see changed but
    I understand that it will not be merged and I will not be listed as a contributor on this project.

In i2svg, when specifying the 'node' param, typescript then requires the 'callback' param. Both 'node' and 'param' should be optional.
@tagliala

Copy link
Copy Markdown
Member

@mlwilkerson could you please take a look here?

@mlwilkerson

Copy link
Copy Markdown
Member

Thanks @cooljeffro and @tagliala . This looks good to me and seems to bring the intended use of i2svg() in line with our own API doc examples.

I'll pass it along for possible inclusion in a future release of fontawesome-svg-core

i2svg(params?: { node: Node; callback: () => void }): Promise<void>;
i2svg(params?: { node?: Node; callback?: () => void }): Promise<void>;
css(): string;
insertCss(): string;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

``

i2svg(params?: { node: Node; callback: () => void }): Promise<void>;
i2svg(params?: { node?: Node; callback?: () => void }): Promise<void>;
css(): string;
insertCss(): string;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

``

@BraianS

BraianS commented Aug 19, 2026

Copy link
Copy Markdown

Any update about it?

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants