-
Notifications
You must be signed in to change notification settings - Fork 365
[Preview NG] Introduce hooks to manage preview communication #3495
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
Merged
Merged
Changes from all commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
fbdeb22
[jer/preview-ng-5-hooks] [jer/preview-hooks] Add usePreviewHost and u…
jeremywiebe 0ae5eb2
[jer/preview-ng-5-hooks] Rename usePreviewHost/Client to usePreviewCo…
jeremywiebe 3ab4eb0
[jer/preview-ng-5-hooks] Unify duplicate createQuestionPreview helper…
jeremywiebe f991b55
[jer/preview-ng-5-hooks] Formatting
jeremywiebe 96830c3
[jer/preview-ng-5-hooks] Inline iframe (only used once)
jeremywiebe c5a587a
[jer/preview-ng-5-hooks] Use '/' for targetOrigin for all postMessage…
jeremywiebe dd854c3
[jer/preview-ng-5-hooks] Remove ESLint suppression and just add ifram…
jeremywiebe f39c505
[jer/preview-ng-5-hooks] Fix naming of UsePreviewHostResult to UsePre…
jeremywiebe a0304bc
[jer/preview-ng-5-hooks] Remove unused 'lint report' message
jeremywiebe 4123c9e
[jer/preview-ng-5-hooks] Fix naming of UsePreviewClientResult to UseP…
jeremywiebe 39b9d17
[jer/preview-ng-5-hooks] Inline iframe var
jeremywiebe e9e3183
[jer/preview-ng-5-hooks] Fix up example in docs
jeremywiebe f7ac1dd
[jer/preview-ng-5-hooks] Remove message id: not actually useful/needed
jeremywiebe 9b16cfb
[jer/preview-ng-5-hooks] Make each hook's message handler the same (u…
jeremywiebe 2097499
[jer/preview-ng-5-hooks] Fixups
jeremywiebe aebd01b
[jer/preview-ng-5-hooks] Remove unneeded Log mock
jeremywiebe 23c4d8b
[jer/preview-ng-5-hooks] Use interfaces for message types (and 'inher…
jeremywiebe 4688928
[jer/preview-ng-5-hooks] Rename request-data event to iframe-ready
jeremywiebe d79d1f9
[jer/preview-ng-5-hooks] Ensure iframe's contentWindow is non-null wh…
jeremywiebe ace7e36
[jer/preview-ng-5-hooks] Make sure we capture pending data if iframe …
jeremywiebe ec89ac8
[jer/preview-ng-5-hooks] Rename event type and migrate to constructor…
jeremywiebe 3992713
[jer/preview-ng-5-hooks] Remove 'paths' from linterContext test data …
jeremywiebe File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@khanacademy/perseus-editor": minor | ||
| --- | ||
|
|
||
| Add usePreviewController and usePreviewPresenter hooks for typed iframe preview communication |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,17 +23,6 @@ interface PreviewMessageBase { | |
| source: typeof PREVIEW_MESSAGE_SOURCE; | ||
| } | ||
|
|
||
| /** | ||
| * Base type for messages with iframe ID | ||
| * | ||
| * Note: The ID is primarily used for debugging/logging purposes. | ||
| * Message routing is handled by event.source filtering in the hooks, | ||
| * not by comparing ID strings. | ||
| */ | ||
| interface PreviewMessageWithId extends PreviewMessageBase { | ||
| id: string; | ||
| } | ||
|
|
||
| /** | ||
| * Data for question preview (full item with question, answer area, and hints) | ||
| */ | ||
|
|
@@ -82,10 +71,10 @@ export type PreviewContent = | |
| /** | ||
| * Message from parent sending content data to iframe | ||
| */ | ||
| type PreviewDataMessage = PreviewMessageWithId & { | ||
| interface PreviewDataMessage extends PreviewMessageBase { | ||
| type: "content-data"; | ||
| content: PreviewContent; | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Union of all messages sent from parent to iframe | ||
|
|
@@ -95,32 +84,30 @@ export type ParentToIframeMessage = PreviewDataMessage; | |
| // ---- Iframe → Parent messages ---- | ||
|
|
||
| /** | ||
| * Message from iframe requesting data from parent | ||
| * Message from iframe to parent telling it the iframe is ready | ||
| */ | ||
| type PreviewDataRequestMessage = PreviewMessageWithId & { | ||
| type: "request-data"; | ||
| }; | ||
| interface PreviewIframeReadyMessage extends PreviewMessageBase { | ||
| type: "iframe-ready"; | ||
| } | ||
|
|
||
| /** | ||
| * Message from iframe reporting its content height | ||
| */ | ||
| type PreviewHeightUpdateMessage = PreviewMessageWithId & { | ||
| interface PreviewHeightUpdateMessage extends PreviewMessageBase { | ||
| type: "height-update"; | ||
| height: number; | ||
| }; | ||
|
|
||
| /** | ||
| * Message from iframe reporting lint warnings | ||
| */ | ||
| type PreviewLintReportMessage = PreviewMessageWithId & { | ||
| type: "lint-report"; | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This feature exists today, but it just logs the lint report to the JS console, so effectively, it does nothing! Also, the frontend doesn't actually send this data so we're waiting for a message that'll never come! We'll bring it back if/when we actually want to implement it. |
||
| lintWarnings: ReadonlyArray<any>; | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Union of all messages sent from iframe to parent | ||
| */ | ||
| export type IframeToParentMessage = | ||
| | PreviewDataRequestMessage | ||
| | PreviewHeightUpdateMessage | ||
| | PreviewLintReportMessage; | ||
| | PreviewIframeReadyMessage | ||
| | PreviewHeightUpdateMessage; | ||
|
|
||
| export function createPreviewIframeReadyMessage(): PreviewIframeReadyMessage { | ||
| return { | ||
| source: PREVIEW_MESSAGE_SOURCE, | ||
| type: "iframe-ready", | ||
| }; | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on Ben's question about why the ID shouldn't be used for routing, I realized that the "gating" by message source that we do in the message handlers is sufficient. The
idis just an extra value that doesn't add any further safety and so is just extra work for no benefit.