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

Correct flex text type definition #311

Merged
merged 1 commit into from
Oct 18, 2021
Merged

Conversation

dlackty
Copy link
Contributor

@dlackty dlackty commented Oct 15, 2021

According to Flex documentation, only one of text or contents needs to be present in flex text.

Be sure to set either one of the text property or contents property. If you set the contents property, text is ignored.

lib/types.ts Outdated Show resolved Hide resolved
lib/types.ts Outdated Show resolved Hide resolved
lib/types.ts Outdated
contents: FlexSpan[];
}

export declare type FlexText = (FlexTextWithText | FlexTextWithContents) &
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
export declare type FlexText = (FlexTextWithText | FlexTextWithContents) &
export type FlexText = (FlexTextWithText | FlexTextWithContents) &

Besides, Typescript will accept both text & contents in one object. The reason is here:
https://stackoverflow.com/questions/37688318/typescript-interface-possible-to-make-one-or-the-other-properties-required

But anyway, thanks for the type definition.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! All the style issues are corrected.

I've read through the stackoverflow post and see an answer describing a way to make Typescript accept only one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants