Skip to content

Types: Allow font to be partial scriptable and individually scriptable - #10364

Merged
etimberg merged 8 commits into
chartjs:masterfrom
LeeLenaleee:types/scriptable-font
May 25, 2022
Merged

etimberg merged 8 commits into
chartjs:masterfrom
LeeLenaleee:types/scriptable-font

Conversation

@LeeLenaleee

Copy link
Copy Markdown
Collaborator

Resolves #10363

@LeeLenaleee LeeLenaleee added the type: types Typescript type changes label May 19, 2022
@LeeLenaleee LeeLenaleee changed the title Types/scriptable font Types: Allow font to be partial scriptable and individually scriptable May 19, 2022
@LeeLenaleee

Copy link
Copy Markdown
Collaborator Author

Not really happy about the approach especially the CanvasFontSpec but didn't know an other/better way to achieve the desired behaviour

@etimberg
etimberg requested a review from kurkle May 21, 2022 13:54

@kurkle kurkle left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the FontSpec should not include scriptability. Could create a ScriptableFontSpec or something if that is needed. The individual props of the font have not always been scriptable, but are they now?

@LeeLenaleee

Copy link
Copy Markdown
Collaborator Author

From the bit of testing I did it seemed like it. But will test it bit more when I get back from vacation in a few days.

Comment thread types/index.esm.d.ts Outdated

@LeeLenaleee LeeLenaleee left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Rename for consitency with ScriptableAndArray

Comment thread types/index.esm.d.ts Outdated
Comment thread types/index.esm.d.ts Outdated
Comment thread types/index.esm.d.ts Outdated
Comment thread types/index.esm.d.ts Outdated
Comment thread types/index.esm.d.ts Outdated
Comment thread types/index.esm.d.ts Outdated
Comment thread types/index.esm.d.ts Outdated
Comment thread types/index.esm.d.ts Outdated
Comment thread types/index.esm.d.ts Outdated
Comment thread types/index.esm.d.ts Outdated

@kurkle kurkle left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

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

Labels

type: types Typescript type changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FontSpec returned by scripted font option requires all properties to be set, while they should be optional

4 participants