-
Notifications
You must be signed in to change notification settings - Fork 128
Fullscreen viz mode + embedded viz sizing/scroll fixes for MCP App #426
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
base: main
Are you sure you want to change the base?
Changes from all commits
08b9473
f2bf247
f2e1752
157b025
69f900b
eb26c4d
df8a719
cca55c0
2378516
c88128b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| { | ||
| "mcp-apps": true | ||
| } |
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,3 @@ | ||
| { | ||
| "mcp-apps": false | ||
| "mcp-apps": true | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,9 @@ export function createTableauVizElement(vizUrl: string, token: string): HTMLElem | |
| // Set the token for authentication | ||
| viz.setAttribute('token', token); | ||
|
|
||
| // Hide the toolbar to prevent inner scrollbar (toolbar adds ~27-40px beyond reported sheet height) | ||
| viz.setAttribute('toolbar', 'hidden'); | ||
|
|
||
| return viz; | ||
| } | ||
|
|
||
|
|
@@ -39,16 +42,24 @@ export function embedTableauViz(vizUrl: string, token: string): void { | |
| const viz = createTableauVizElement(vizUrl, token); | ||
|
|
||
| // The Embedding API reports the viz's natural size via `firstvizsizeknown`. | ||
| // Set the element's height to the viz's natural sheet height so it renders | ||
| // fully; the ext-apps SDK's built-in auto-resize then grows the app frame to | ||
| // match the resulting document height. | ||
| // Set the element's width/height ATTRIBUTES (not just CSS) so the Embedding | ||
| // API sizes its internal cross-origin iframe correctly. Without attributes, | ||
| // the iframe uses a default size and creates its own scrollbar. | ||
| // Width is set to "100%" for responsive behavior; height is the native px value. | ||
| viz.addEventListener('firstvizsizeknown', (event) => { | ||
| const vizSize = (event as CustomEvent).detail?.vizSize; | ||
| const sheetHeight = vizSize?.sheetSize?.maxSize?.height; | ||
| if (typeof sheetHeight !== 'number') { | ||
| return; | ||
| } | ||
|
|
||
| console.log('height: ', sheetHeight); | ||
|
Contributor
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. ℹ️ Info · 🧠 Logic A Tip Suggested fix Remove the Review by CodeNod |
||
|
|
||
| // Set attributes for Embedding API iframe sizing | ||
| viz.setAttribute('width', '100%'); | ||
| viz.setAttribute('height', String(sheetHeight)); | ||
|
|
||
| // Also set CSS height for backward compatibility | ||
| viz.style.height = `${sheetHeight}px`; | ||
| }); | ||
|
|
||
|
|
||
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.
ℹ️ Info · 🎨 Style
The
mcp-appsfeature flag is flipped totruein what appears to be a shared config file. If this is intended only for development/testing, shipping it enabled by default may unintentionally expose the feature.Tip
Suggested fix
Confirm this flag change is intentional for production; if not, revert or gate behind an environment-specific mechanism.
Review by CodeNod