Skip to content

Conversation

@chrisjtang
Copy link
Contributor

@chrisjtang chrisjtang commented Oct 12, 2022

Description

This change includes typing for the DiscussionEmbedConfig interface to handle SSO properties.

Motivation and Context

The DiscussionEmbed component's SSO capabilities should be compatible with TypeScript.

How Has This Been Tested?

Tested on a Disqus-React repo running TS locally. Also tested with a Disqus publisher's config that is running TS (details in DSQDEV-2316). Previously, there were TS error messages being generated in the pub's config, but after this change, the error messages were no longer appearing.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • All new and existing tests passed.

@chrisjtang chrisjtang self-assigned this Oct 12, 2022
@chrisjtang chrisjtang changed the title Add TS interfaces for DiscussionEmbedConfig for SSO Add TS compatibility for DiscussionEmbedConfig interface for SSO Oct 12, 2022
@chrisjtang chrisjtang marked this pull request as ready for review October 13, 2022 18:05
@chrisjtang chrisjtang requested a review from Hovspian October 13, 2022 18:05
Copy link

@Hovspian Hovspian left a comment

Choose a reason for hiding this comment

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

lgtm

@chrisjtang chrisjtang merged commit 22cbc06 into master Oct 13, 2022
@chrisjtang chrisjtang deleted the sso-embed-typescript branch October 13, 2022 20:48
This was referenced Sep 7, 2024
This was referenced Sep 17, 2024
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.

3 participants