diff --git a/e2e/test/scenarios/dashboard/dashboard.cy.spec.js b/e2e/test/scenarios/dashboard/dashboard.cy.spec.js index aa2509b72f5a80133d1ed15e665cc4f7e9337a72..c3c647313cf880efc1842d0aace242417ff03956 100644 --- a/e2e/test/scenarios/dashboard/dashboard.cy.spec.js +++ b/e2e/test/scenarios/dashboard/dashboard.cy.spec.js @@ -20,6 +20,8 @@ import { enableTracking, expectGoodSnowplowEvent, closeNavigationSidebar, + updateDashboardCards, + getDashboardCards, } from "e2e/support/helpers"; import { SAMPLE_DB_ID } from "e2e/support/cypress_data"; @@ -791,6 +793,23 @@ describe("scenarios > dashboard", () => { cy.icon("pencil").should("be.visible"); }); + + it("shows sorted cards on mobile screens", () => { + cy.viewport(400, 800); + cy.createDashboard().then(({ body: { id: dashboard_id } }) => { + const cards = [ + createTextCard("bottom", 1), // the bottom card intentionally goes first to have unsorted cards coming from the BE + createTextCard("top", 0), + ]; + + updateDashboardCards({ dashboard_id, cards }); + + visitDashboard(dashboard_id); + }); + + getDashboardCards().eq(0).contains("top"); + getDashboardCards().eq(1).contains("bottom"); + }); }); describeWithSnowplow("scenarios > dashboard", () => { @@ -898,3 +917,21 @@ function createDashboardUsingUI(name, description) { cy.url().should("contain", `/dashboard/${body.id}`); }); } + +function createTextCard(text, row) { + return { + row, + size_x: 24, + size_y: 1, + visualization_settings: { + virtual_card: { + name: null, + display: "text", + visualization_settings: {}, + dataset_query: {}, + archived: false, + }, + text, + }, + }; +} diff --git a/e2e/test/scenarios/dashboard/x-rays.cy.spec.js b/e2e/test/scenarios/dashboard/x-rays.cy.spec.js index 47863be0f3c06acd08772ff5f116da4907708c7d..7548e7ebe01f06691f7bf95aedfbc2089e613c0f 100644 --- a/e2e/test/scenarios/dashboard/x-rays.cy.spec.js +++ b/e2e/test/scenarios/dashboard/x-rays.cy.spec.js @@ -9,6 +9,7 @@ import { addOrUpdateDashboardCard, visitDashboardAndCreateTab, popover, + getDashboardCards, } from "e2e/support/helpers"; import { SAMPLE_DB_ID } from "e2e/support/cypress_data"; @@ -189,6 +190,33 @@ describe("scenarios > x-rays", () => { cy.findByText("How these transactions are distributed"); }); + it("should start loading cards from top to bottom", () => { + // to check the order of loaded cards this test lets the first intercepted + // request to be resolved successfully and then it fails all others + + const totalRequests = 8; + const successfullyLoadedCards = 1; + const failedCards = totalRequests - successfullyLoadedCards; + + cy.intercept({ + method: "POST", + url: "/api/dataset", + times: successfullyLoadedCards, + }).as("dataset"); + + cy.intercept( + { method: "POST", url: "/api/dataset", times: failedCards }, + { statusCode: 500 }, + ).as("datasetFailed"); + + cy.visit(`/auto/dashboard/table/${ORDERS_ID}`); + + cy.wait("@dataset"); + cy.wait("@datasetFailed"); + + getDashboardCards().eq(1).contains("Total transactions"); + }); + it("should be able to click the title of an x-ray dashcard to see it in the query builder (metabase#19405)", () => { const timeout = { timeout: 10000 }; diff --git a/frontend/src/metabase/dashboard/components/grid/utils.js b/frontend/src/metabase/dashboard/components/grid/utils.js index 58c3e75a1c2edc86d937dfa60bba73d10866d5e5..5aa72bec6bfb9c65c70d62be7aefb3c82ddcf13d 100644 --- a/frontend/src/metabase/dashboard/components/grid/utils.js +++ b/frontend/src/metabase/dashboard/components/grid/utils.js @@ -5,24 +5,13 @@ function sumVerticalSpace(layout) { return layout.reduce((sum, current) => sum + current.h, 0); } -export function sortCardsForMobile(a, b) { - const yDiff = a.y - b.y; - - // sort by y position first - if (yDiff !== 0) { - return yDiff; - } - - // for items on the same row, sort by x position - return a.x - b.x; -} export function generateMobileLayout({ desktopLayout, defaultCardHeight, heightByDisplayType = {}, }) { const mobile = []; - desktopLayout.sort(sortCardsForMobile).forEach(item => { + desktopLayout.forEach(item => { const card = item.dashcard.card; const height = heightByDisplayType[card.display] || defaultCardHeight; mobile.push({ diff --git a/frontend/src/metabase/dashboard/components/grid/utils.unit.spec.js b/frontend/src/metabase/dashboard/components/grid/utils.unit.spec.js deleted file mode 100644 index 22781b111089cd7da7022c4dda4fbe4a773c9d78..0000000000000000000000000000000000000000 --- a/frontend/src/metabase/dashboard/components/grid/utils.unit.spec.js +++ /dev/null @@ -1,36 +0,0 @@ -import { sortCardsForMobile } from "./utils"; - -describe("Dashcard > grid > utils", () => { - describe("sortCardsForMobile", () => { - it("should sort cards by y position first", () => { - const a = { id: "top", y: 1, x: 3 }; - const b = { id: "middle", y: 2, x: 2 }; - const c = { id: "bottom", y: 3, x: 1 }; - - const result = [b, a, c].sort(sortCardsForMobile); - expect(result).toEqual([a, b, c]); - }); - - it("should sort cards by x position if y position is the same", () => { - const a = { id: "left", y: 5, x: 1 }; - const b = { id: "middle", y: 5, x: 2 }; - const c = { id: "right", y: 5, x: 3 }; - - const result = [c, a, b].sort(sortCardsForMobile); - expect(result).toEqual([a, b, c]); - }); - - it("should sort cards by x and y positions", () => { - const a = { id: "top", y: 1, x: 3 }; - const b = { id: "middle", y: 2, x: 2 }; - const c = { id: "bottom", y: 3, x: 1 }; - - const d = { id: "left", y: 5, x: 1 }; - const e = { id: "middle", y: 5, x: 2 }; - const f = { id: "right", y: 5, x: 3 }; - - const result = [f, d, c, a, b, e].sort(sortCardsForMobile); - expect(result).toEqual([a, b, c, d, e, f]); - }); - }); -}); diff --git a/frontend/src/metabase/dashboard/selectors.unit.spec.js b/frontend/src/metabase/dashboard/selectors.unit.spec.js index 8695da069aaccb755ca20ae644d7eb6e99ee9215..26889f28316029deceedec27001e8ee5f0492bb0 100644 --- a/frontend/src/metabase/dashboard/selectors.unit.spec.js +++ b/frontend/src/metabase/dashboard/selectors.unit.spec.js @@ -7,6 +7,7 @@ import { getEditingParameterId, getIsEditingParameter, getClickBehaviorSidebarDashcard, + getDashboardComplete, } from "metabase/dashboard/selectors"; import { createMockSettingsState } from "metabase-types/store/mocks"; import Field from "metabase-lib/metadata/Field"; @@ -340,4 +341,65 @@ describe("dashboard/selectors", () => { ); }); }); + + describe("getDashboardComplete", () => { + const multiCardState = chain(STATE) + .assocIn(["dashboard", "dashboards", 0, "ordered_cards"], [0, 1, 2]) + .assocIn(["dashboard", "dashcards", 2], { + card: { id: 2, dataset_query: { type: "query", query: {} } }, + parameter_mappings: [], + }) + .value(); + + const setup = positions => { + const newStateChain = chain(multiCardState); + + positions.forEach((position, index) => { + newStateChain + .assocIn(["dashboard", "dashcards", index, "row"], position.row) + .assocIn(["dashboard", "dashcards", index, "col"], position.col); + }); + + return newStateChain.value(); + }; + + it("should filter out removed dashcards", () => { + const state = chain(multiCardState) + .assocIn(["dashboard", "dashcards", 0, "isRemoved"], true) + .assocIn(["dashboard", "dashcards", 2, "isRemoved"], true) + .value(); + + expect(getDashboardComplete(state).ordered_cards).toEqual([ + multiCardState.dashboard.dashcards[1], + ]); + }); + + it("should sort cards based on their positions top to bottom", () => { + const state = setup([ + { row: 2, col: 0 }, + { row: 1, col: 1 }, + { row: 0, col: 2 }, + ]); + + const cards = getDashboardComplete(state).ordered_cards; + + expect(cards[2].card.id).toBe(0); + expect(cards[1].card.id).toBe(1); + expect(cards[0].card.id).toBe(2); + }); + + it("should sort cards based on their positions left to right when on the same row", () => { + const state = setup([ + { row: 0, col: 2 }, + { row: 0, col: 1 }, + { row: 0, col: 0 }, + ]); + + const cards = getDashboardComplete(state).ordered_cards; + + expect(cards[2].card.id).toBe(0); + expect(cards[1].card.id).toBe(1); + expect(cards[0].card.id).toBe(2); + }); + }); }); diff --git a/frontend/src/metabase/dashboard/selectors/selectors.js b/frontend/src/metabase/dashboard/selectors/selectors.js index 5dc68b1b9c8b6b3bd8200e1018fc88b18545a6e4..c97d92c10c76ef1470c70f9f5d7d4c07f4b45f0a 100644 --- a/frontend/src/metabase/dashboard/selectors/selectors.js +++ b/frontend/src/metabase/dashboard/selectors/selectors.js @@ -115,13 +115,33 @@ export const getDashCardTable = (state, dashcardId) => { export const getDashboardComplete = createSelector( [getDashboard, getDashcards], - (dashboard, dashcards) => - dashboard && { - ...dashboard, - ordered_cards: dashboard.ordered_cards - .map(id => dashcards[id]) - .filter(dc => !dc.isRemoved), - }, + (dashboard, dashcards) => { + if (!dashboard) { + return null; + } + + const ordered_cards = dashboard.ordered_cards + .map(id => dashcards[id]) + .filter(dc => !dc.isRemoved) + .sort((a, b) => { + const rowDiff = a.row - b.row; + + // sort by y position first + if (rowDiff !== 0) { + return rowDiff; + } + + // for items on the same row, sort by x position + return a.col - b.col; + }); + + return ( + dashboard && { + ...dashboard, + ordered_cards, + } + ); + }, ); export const getAutoApplyFiltersToastId = state =>