Conversation
|
|
Coverage Impact This PR will not change total coverage. Modified Files with Diff Coverage (1)
🛟 Help
|
1 new issue
|
There was a problem hiding this comment.
Pull request overview
This PR is a small SDK cleanup focused on improving the embedded messaging developer experience by exporting missing types and making embedded APIs/components easier to discover via clearer JSDoc examples.
Changes:
- Export
IterableEmbeddedViewPropsfrom the package entrypoint. - Add/expand JSDoc for embedded components and helpers (including fenced code blocks for examples).
- Minor formatting cleanup in embedded component code.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/index.tsx | Re-exports IterableEmbeddedViewProps from the embedded module. |
| src/embedded/hooks/useEmbeddedView/useEmbeddedView.ts | Wraps the existing usage example in a fenced tsx code block. |
| src/embedded/hooks/useEmbeddedView/getStyles.ts | Wraps the existing usage example in a fenced ts code block. |
| src/embedded/hooks/useEmbeddedView/getMedia.ts | Wraps the existing usage example in a fenced ts code block. |
| src/embedded/components/IterableEmbeddedView.tsx | Exports IterableEmbeddedViewProps and adds a detailed usage example in docs. |
| src/embedded/components/IterableEmbeddedNotification/IterableEmbeddedNotification.tsx | Adds JSDoc with a usage example. |
| src/embedded/components/IterableEmbeddedCard/IterableEmbeddedCard.tsx | Replaces TODO doc with proper JSDoc and applies small formatting cleanup. |
| src/embedded/components/IterableEmbeddedBanner/IterableEmbeddedBanner.tsx | Replaces TODO doc with proper JSDoc and adds a usage example. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… and links to OOTB views
…rface for clarity
joaodordio
left a comment
joaodordio
left a comment
There was a problem hiding this comment.
Left too minor comments for
Improving clarity. But otherwise LGTM
| * metadata: { | ||
| * messageId: 'test-message-123', | ||
| * placementId: 101, | ||
| * campaignId: 123456, |
There was a problem hiding this comment.
The type IterableEmbeddedMessageMetadata has campaignId?: number | null. The example implies it’s required. Minor, but could confuse developers.
| * placementId: 101, | ||
| * campaignId: 123456, | ||
| * }, | ||
| * elements: { |
There was a problem hiding this comment.
The type IterableEmbeddedMessage has elements?: IterableEmbeddedMessageElements | null.
Same issue, a message with no elements is valid (e.g. a metadata-only message), but the example doesn’t reflect that.
There was a problem hiding this comment.
It's not exactly required for the view, but it would result in a very strange looking component as everything would be empty.
🔹 JIRA Ticket(s) if any
✏️ Description
This is just a cleanup task. Comments are cleaned up and documentation is added.