Skip to content
Snippets Groups Projects
Unverified Commit da628b52 authored by metabase-bot[bot]'s avatar metabase-bot[bot] Committed by GitHub
Browse files

:robot: backported "Fix embedding" (#40969)


* Fix embedding (#40841)

* Fix overflow static dashboard

* Fix `frame` event's height for interactive dashboards

* Fix E2E tests

* Fix test names to match the correct GitHub issue numbers

* Use `data-element-id` for consistency

* Make the test selector more consistent

* Use the new backported util from master

---------

Co-authored-by: default avatarMahatthana (Kelvin) Nomsawadi <me@bboykelvin.dev>
Co-authored-by: default avatarMahatthana Nomsawadi <mahatthana.n@gmail.com>
parent 9f2124c3
No related branches found
No related tags found
No related merge requests found
Showing
with 194 additions and 49 deletions
...@@ -261,3 +261,16 @@ export function createPublicQuestionLink(questionId) { ...@@ -261,3 +261,16 @@ export function createPublicQuestionLink(questionId) {
export function createPublicDashboardLink(dashboardId) { export function createPublicDashboardLink(dashboardId) {
return cy.request("POST", `/api/dashboard/${dashboardId}/public_link`, {}); return cy.request("POST", `/api/dashboard/${dashboardId}/public_link`, {});
} }
export const visitFullAppEmbeddingUrl = ({ url, qs, onBeforeLoad }) => {
cy.visit({
url,
qs,
onBeforeLoad(window) {
// cypress runs all tests in an iframe and the app uses this property to avoid embedding mode for all tests
// by removing the property the app would work in embedding mode
window.Cypress = undefined;
onBeforeLoad?.(window);
},
});
};
...@@ -201,5 +201,5 @@ export const undoToastList = () => { ...@@ -201,5 +201,5 @@ export const undoToastList = () => {
}; };
export function dashboardCards() { export function dashboardCards() {
return cy.get("#Dashboard-Cards-Container"); return cy.get("[data-element-id=dashboard-cards-container]");
} }
import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database"; import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database";
import { restore } from "e2e/support/helpers"; import { restore, visitFullAppEmbeddingUrl } from "e2e/support/helpers";
// Couldn't import from `metabase/components/ExplicitSize` because dependency issue. // Couldn't import from `metabase/components/ExplicitSize` because dependency issue.
// It will fail Cypress tests. // It will fail Cypress tests.
...@@ -102,18 +102,3 @@ describe("issue 29304", () => { ...@@ -102,18 +102,3 @@ describe("issue 29304", () => {
}); });
}); });
}); });
// Use full-app embedding to test because `ExplicitSize` checks for `isCypressActive`,
// which checks `window.Cypress`, and will disable the refresh mode on Cypress test.
// If we test by simply visiting the dashboard, the refresh mode will be disabled,
// and we won't be able to reproduce the problem.
const visitFullAppEmbeddingUrl = ({ url }) => {
cy.visit({
url,
onBeforeLoad(window) {
// cypress runs all tests in an iframe and the app uses this property to avoid embedding mode for all tests
// by removing the property the app would work in embedding mode
window.Cypress = undefined;
},
});
};
...@@ -19,6 +19,7 @@ import { ...@@ -19,6 +19,7 @@ import {
undoToast, undoToast,
visitDashboard, visitDashboard,
visitEmbeddedPage, visitEmbeddedPage,
visitFullAppEmbeddingUrl,
visitPublicDashboard, visitPublicDashboard,
} from "e2e/support/helpers"; } from "e2e/support/helpers";
...@@ -729,15 +730,3 @@ const openSlowFullAppEmbeddingDashboard = (params = {}) => { ...@@ -729,15 +730,3 @@ const openSlowFullAppEmbeddingDashboard = (params = {}) => {
getDashboardCard().should("be.visible"); getDashboardCard().should("be.visible");
}; };
const visitFullAppEmbeddingUrl = ({ url, qs }) => {
cy.visit({
url,
qs,
onBeforeLoad(window) {
// cypress runs all tests in an iframe and the app uses this property to avoid embedding mode for all tests
// by removing the property the app would work in embedding mode
window.Cypress = undefined;
},
});
};
...@@ -18,8 +18,12 @@ import { ...@@ -18,8 +18,12 @@ import {
dashboardGrid, dashboardGrid,
createDashboardWithTabs, createDashboardWithTabs,
goToTab, goToTab,
visitFullAppEmbeddingUrl,
} from "e2e/support/helpers"; } from "e2e/support/helpers";
import { createMockDashboardCard } from "metabase-types/api/mocks"; import {
createMockDashboardCard,
createMockTextDashboardCard,
} from "metabase-types/api/mocks";
const { ORDERS } = SAMPLE_DATABASE; const { ORDERS } = SAMPLE_DATABASE;
...@@ -458,6 +462,79 @@ describeEE("scenarios > embedding > full app", () => { ...@@ -458,6 +462,79 @@ describeEE("scenarios > embedding > full app", () => {
}, },
); );
}); });
it("should send `frame` message with dashboard height when the dashboard is resized (metabase#37437)", () => {
const TAB_1 = { id: 1, name: "Tab 1" };
const TAB_2 = { id: 2, name: "Tab 2" };
createDashboardWithTabs({
tabs: [TAB_1, TAB_2],
name: "Dashboard",
dashcards: [
createMockTextDashboardCard({
dashboard_tab_id: TAB_1.id,
size_x: 10,
size_y: 20,
text: "I am a text card",
}),
],
}).then(dashboard => {
visitFullAppEmbeddingUrl({
url: `/dashboard/${dashboard.id}`,
onBeforeLoad(window) {
cy.spy(window.parent, "postMessage").as("postMessage");
},
});
});
cy.get("@postMessage")
.should("have.been.calledWith", {
metabase: {
type: "frame",
frame: {
mode: "fit",
height: Cypress.sinon.match(value => value > 1000),
},
},
})
.and("not.have.been.calledWith", {
metabase: {
type: "frame",
frame: {
mode: "fit",
height: Cypress.sinon.match(value => value < 400),
},
},
});
cy.findByRole("tab", { name: TAB_2.name }).click();
cy.get("@postMessage").should("have.been.calledWith", {
metabase: {
type: "frame",
frame: {
mode: "fit",
height: Cypress.sinon.match(value => value < 400),
},
},
});
cy.findByTestId("app-bar").findByText("Our analytics").click();
cy.findByRole("heading", { name: "Metabase analytics" }).should(
"be.visible",
);
cy.get("@postMessage").then(postMessage => {
expect(
postMessage.lastCall.calledWith({
metabase: {
type: "frame",
frame: {
mode: "fit",
height: 800,
},
},
}),
).to.be.true;
});
});
}); });
describe("x-ray dashboards", () => { describe("x-ray dashboards", () => {
...@@ -480,18 +557,6 @@ describeEE("scenarios > embedding > full app", () => { ...@@ -480,18 +557,6 @@ describeEE("scenarios > embedding > full app", () => {
}); });
}); });
const visitFullAppEmbeddingUrl = ({ url, qs }) => {
cy.visit({
url,
qs,
onBeforeLoad(window) {
// cypress runs all tests in an iframe and the app uses this property to avoid embedding mode for all tests
// by removing the property the app would work in embedding mode
window.Cypress = undefined;
},
});
};
const visitQuestionUrl = urlOptions => { const visitQuestionUrl = urlOptions => {
visitFullAppEmbeddingUrl(urlOptions); visitFullAppEmbeddingUrl(urlOptions);
cy.wait("@getCardQuery"); cy.wait("@getCardQuery");
......
import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database";
import {
getIframeBody,
openStaticEmbeddingModal,
restore,
updateDashboardCards,
visitDashboard,
} from "e2e/support/helpers";
const { PRODUCTS_ID } = SAMPLE_DATABASE;
const questionDetails = {
name: "Products",
query: { "source-table": PRODUCTS_ID, limit: 2 },
};
const dashboardDetails = {
name: "long dashboard",
enable_embedding: true,
};
describe("issue 40660", () => {
beforeEach(() => {
cy.intercept("GET", "/api/preview_embed/dashboard/**").as(
"previewDashboard",
);
cy.intercept("GET", "/api/dashboard/**/params/**/values").as("values");
restore();
cy.signInAsAdmin();
cy.createQuestionAndDashboard({
questionDetails,
dashboardDetails,
}).then(({ body: { id, card_id, dashboard_id } }) => {
updateDashboardCards({
dashboard_id,
cards: [{ card_id }, { card_id }, { card_id }],
});
visitDashboard(dashboard_id);
});
});
it("static dashboard content shouldn't overflow its container (metabase#40660)", () => {
openStaticEmbeddingModal({
activeTab: "parameters",
previewMode: "preview",
});
getIframeBody().within(() => {
cy.findByTestId("embed-frame").scrollTo("bottom");
cy.findByRole("link", { name: "Powered by Metabase" }).should(
"be.visible",
);
});
});
});
...@@ -131,14 +131,15 @@ export const createMockVirtualDashCard = ( ...@@ -131,14 +131,15 @@ export const createMockVirtualDashCard = (
}; };
}; };
export const createMockTextDashboardCard = ( export const createMockTextDashboardCard = ({
opts?: VirtualDashboardCardOpts & { text?: string }, text,
): VirtualDashboardCard => ...opts
}: VirtualDashboardCardOpts & { text?: string } = {}): VirtualDashboardCard =>
createMockVirtualDashCard({ createMockVirtualDashCard({
...opts, ...opts,
card: createMockVirtualCard({ display: "text" }), card: createMockVirtualCard({ display: "text" }),
visualization_settings: { visualization_settings: {
text: opts?.text ?? "Body Text", text: text ?? "Body Text",
}, },
}); });
......
...@@ -139,6 +139,9 @@ export const ParametersAndCardsContainer = styled.div<{ ...@@ -139,6 +139,9 @@ export const ParametersAndCardsContainer = styled.div<{
overflow-x: clip; overflow-x: clip;
} }
padding-bottom: 40px; padding-bottom: 40px;
/* Makes sure it doesn't use all the height, so the actual content height could be used in embedding #37437 */
align-self: ${({ shouldMakeDashboardHeaderStickyAfterScrolling }) =>
!shouldMakeDashboardHeaderStickyAfterScrolling && "flex-start"};
&.${SAVING_DOM_IMAGE_CLASS} { &.${SAVING_DOM_IMAGE_CLASS} {
${ParametersWidgetContainer} { ${ParametersWidgetContainer} {
......
...@@ -533,6 +533,8 @@ function DashboardInner(props: DashboardProps) { ...@@ -533,6 +533,8 @@ function DashboardInner(props: DashboardProps) {
{() => ( {() => (
<DashboardStyled> <DashboardStyled>
<DashboardHeaderContainer <DashboardHeaderContainer
data-element-id="dashboard-header-container"
id="Dashboard-Header-Container"
isFullscreen={isFullscreen} isFullscreen={isFullscreen}
isNightMode={shouldRenderAsNightMode} isNightMode={shouldRenderAsNightMode}
> >
...@@ -553,13 +555,14 @@ function DashboardInner(props: DashboardProps) { ...@@ -553,13 +555,14 @@ function DashboardInner(props: DashboardProps) {
<DashboardBody isEditingOrSharing={isEditing || isSharing}> <DashboardBody isEditingOrSharing={isEditing || isSharing}>
<ParametersAndCardsContainer <ParametersAndCardsContainer
id={DASHBOARD_PDF_EXPORT_ROOT_ID} id={DASHBOARD_PDF_EXPORT_ROOT_ID}
data-element-id="dashboard-parameters-and-cards"
data-testid="dashboard-parameters-and-cards" data-testid="dashboard-parameters-and-cards"
shouldMakeDashboardHeaderStickyAfterScrolling={ shouldMakeDashboardHeaderStickyAfterScrolling={
!isFullscreen && (isEditing || isSharing) !isFullscreen && (isEditing || isSharing)
} }
> >
{renderParameterList()} {renderParameterList()}
<CardsContainer id="Dashboard-Cards-Container"> <CardsContainer data-element-id="dashboard-cards-container">
{renderContent()} {renderContent()}
</CardsContainer> </CardsContainer>
</ParametersAndCardsContainer> </ParametersAndCardsContainer>
......
...@@ -59,8 +59,30 @@ function sendMessage(message) { ...@@ -59,8 +59,30 @@ function sendMessage(message) {
function getFrameSpec() { function getFrameSpec() {
if (isFitViewportMode()) { if (isFitViewportMode()) {
return { mode: "fit", height: document.body.scrollHeight }; return { mode: "fit", height: getScrollHeight() };
} else { } else {
return { mode: "normal", height: document.body.scrollHeight }; return { mode: "normal", height: document.body.scrollHeight };
} }
} }
function defaultGetScrollHeight() {
return document.body.scrollHeight;
}
function getScrollHeight() {
const appBarHeight =
document.getElementById("[data-element-id=app-bar]")?.offsetHeight ?? 0;
const dashboardHeaderHeight =
document.querySelector("[data-element-id=dashboard-header-container]")
?.offsetHeight ?? 0;
const dashboardContentHeight =
document.querySelector("[data-element-id=dashboard-parameters-and-cards]")
?.scrollHeight ?? 0;
const dashboardHeight = dashboardHeaderHeight + dashboardContentHeight;
if (dashboardHeight > 0) {
return appBarHeight + dashboardHeight;
}
return defaultGetScrollHeight();
}
...@@ -28,7 +28,11 @@ const AppBar = (props: AppBarProps): JSX.Element => { ...@@ -28,7 +28,11 @@ const AppBar = (props: AppBarProps): JSX.Element => {
const isSmallScreen = useIsSmallScreen(); const isSmallScreen = useIsSmallScreen();
return ( return (
<AppBarRoot data-testid="app-bar" aria-label={t`Navigation bar`}> <AppBarRoot
data-element-id="app-bar"
data-testid="app-bar"
aria-label={t`Navigation bar`}
>
<ErrorBoundary> <ErrorBoundary>
{isSmallScreen ? ( {isSmallScreen ? (
<AppBarSmall {...props} /> <AppBarSmall {...props} />
......
...@@ -17,6 +17,7 @@ export const Root = styled.div<{ ...@@ -17,6 +17,7 @@ export const Root = styled.div<{
}>` }>`
display: flex; display: flex;
flex-direction: column; flex-direction: column;
overflow: auto;
${props => ${props =>
props.hasScroll && props.hasScroll &&
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment