diff --git a/frontend/src/metabase/collections/components/CollectionLink.jsx b/frontend/src/metabase/collections/components/CollectionLink.jsx index 63ac5bdef00b49919ab83de8d61d685b4e97b952..2c690e5f8ad7347dd0eab88c22d175f20871476b 100644 --- a/frontend/src/metabase/collections/components/CollectionLink.jsx +++ b/frontend/src/metabase/collections/components/CollectionLink.jsx @@ -1,8 +1,8 @@ import styled, { css } from "styled-components"; import Link from "metabase/components/Link"; import { color } from "metabase/lib/colors"; - -import { SIDEBAR_SPACER } from "../constants"; +import { space } from "metabase/styled-components/theme"; +import { SIDEBAR_SPACER } from "metabase/collections/constants"; const dimmedIconCss = css` fill: ${color("white")}; @@ -17,9 +17,9 @@ const CollectionLink = styled(Link)` // now pad it by the depth so we get hover states that are the full width of the sidebar props.depth * (SIDEBAR_SPACER * 2) + SIDEBAR_SPACER}px; position: relative; - padding-right: 8px; - padding-top: 8px; - padding-bottom: 8px; + padding-right: ${space(1)}; + padding-top: ${space(1)}; + padding-bottom: ${space(1)}; display: flex; flex-shrink: 0; align-items: center; diff --git a/frontend/src/metabase/collections/containers/CollectionSidebar/CollectionSidebar.jsx b/frontend/src/metabase/collections/containers/CollectionSidebar/CollectionSidebar.jsx index 8e009d2e5e9b53891f39e54db7915824743f4e6f..ac3beec77459f3c4328f520aadf39d247c174449 100644 --- a/frontend/src/metabase/collections/containers/CollectionSidebar/CollectionSidebar.jsx +++ b/frontend/src/metabase/collections/containers/CollectionSidebar/CollectionSidebar.jsx @@ -74,13 +74,18 @@ class CollectionSidebar extends React.Component { <Collection.Loader id="root"> {({ collection: root }) => ( - <RootCollectionLink isRoot={isRoot} root={root} /> + <RootCollectionLink + handleToggleMobileSidebar={handleToggleMobileSidebar} + isRoot={isRoot} + root={root} + /> )} </Collection.Loader> <Collections collectionId={collectionId} currentUserId={currentUser.id} + handleToggleMobileSidebar={handleToggleMobileSidebar} list={list} onClose={this.onClose} onOpen={this.onOpen} diff --git a/frontend/src/metabase/collections/containers/CollectionSidebar/CollectionSidebarHeader/CollectionSidebarHeader.jsx b/frontend/src/metabase/collections/containers/CollectionSidebar/CollectionSidebarHeader/CollectionSidebarHeader.jsx deleted file mode 100644 index 270abd2c6ebf351bbad7a381a77278a5b1c21eaa..0000000000000000000000000000000000000000 --- a/frontend/src/metabase/collections/containers/CollectionSidebar/CollectionSidebarHeader/CollectionSidebarHeader.jsx +++ /dev/null @@ -1,36 +0,0 @@ -import React from "react"; -import PropTypes from "prop-types"; -import { t } from "ttag"; - -import * as Urls from "metabase/lib/urls"; - -import CollectionDropTarget from "metabase/containers/dnd/CollectionDropTarget"; -import CollectionLink from "metabase/collections/components/CollectionLink"; - -import { Container } from "./CollectionSidebarHeader.styled"; - -const propTypes = { - isRoot: PropTypes.bool.isRequired, - root: PropTypes.object.isRequired, -}; - -export default function CollectionSidebarHeader({ isRoot, root }) { - return ( - <Container> - <CollectionDropTarget collection={root}> - {({ highlighted, hovered }) => ( - <CollectionLink - to={Urls.collection({ id: "root" })} - selected={isRoot} - highlighted={highlighted} - hovered={hovered} - > - {t`Our analytics`} - </CollectionLink> - )} - </CollectionDropTarget> - </Container> - ); -} - -CollectionSidebarHeader.propTypes = propTypes; diff --git a/frontend/src/metabase/collections/containers/CollectionSidebar/CollectionSidebarHeader/CollectionSidebarHeader.styled.jsx b/frontend/src/metabase/collections/containers/CollectionSidebar/CollectionSidebarHeader/CollectionSidebarHeader.styled.jsx deleted file mode 100644 index ef0b6cadc6e88475d101d0b327ca779dc589eed5..0000000000000000000000000000000000000000 --- a/frontend/src/metabase/collections/containers/CollectionSidebar/CollectionSidebarHeader/CollectionSidebarHeader.styled.jsx +++ /dev/null @@ -1,9 +0,0 @@ -import styled from "styled-components"; -import { Box } from "grid-styled"; - -import { space } from "metabase/styled-components/theme"; - -export const Container = styled(Box)` - margin-bottom: ${space(1)}; - margin-top: ${space(2)}; -`; diff --git a/frontend/src/metabase/collections/containers/CollectionSidebar/CollectionSidebarHeader/CollectionSidebarHeader.unit.spec.js b/frontend/src/metabase/collections/containers/CollectionSidebar/CollectionSidebarHeader/CollectionSidebarHeader.unit.spec.js deleted file mode 100644 index 4f4395ce06a94543bf744705fb9ab39b870d73e7..0000000000000000000000000000000000000000 --- a/frontend/src/metabase/collections/containers/CollectionSidebar/CollectionSidebarHeader/CollectionSidebarHeader.unit.spec.js +++ /dev/null @@ -1,19 +0,0 @@ -import React from "react"; -import { render, screen } from "@testing-library/react"; -import { DragDropContextProvider } from "react-dnd"; -import HTML5Backend from "react-dnd-html5-backend"; - -import CollectionSidebarHeader from "./CollectionSidebarHeader"; - -it("displays link to main collection: Our Analytics", () => { - render( - <DragDropContextProvider backend={HTML5Backend}> - <CollectionSidebarHeader - isRoot={false} - root={{ name: "name", id: "root" }} - /> - </DragDropContextProvider>, - ); - - screen.getByText("Our analytics"); -}); diff --git a/frontend/src/metabase/collections/containers/CollectionSidebar/Collections/Collections.jsx b/frontend/src/metabase/collections/containers/CollectionSidebar/Collections/Collections.jsx index 059f93f7ab179e31b82877af03fd35a59adf42c7..7e8fadfd4548df6f1b118708407ecd012ec40ed1 100644 --- a/frontend/src/metabase/collections/containers/CollectionSidebar/Collections/Collections.jsx +++ b/frontend/src/metabase/collections/containers/CollectionSidebar/Collections/Collections.jsx @@ -13,6 +13,7 @@ import { Container } from "./Collections.styled"; const propTypes = { collectionId: PropTypes.number, currentUserId: PropTypes.number, + handleToggleMobileSidebar: PropTypes.func.isRequired, list: PropTypes.array, onClose: PropTypes.func.isRequired, onOpen: PropTypes.func.isRequired, @@ -22,6 +23,7 @@ const propTypes = { export default function Collections({ collectionId, currentUserId, + handleToggleMobileSidebar, list, onClose, onOpen, @@ -39,6 +41,7 @@ export default function Collections({ return ( <Container> <CollectionsList + handleToggleMobileSidebar={handleToggleMobileSidebar} openCollections={openCollections} onClose={onClose} onOpen={onOpen} @@ -49,6 +52,7 @@ export default function Collections({ <Box> <CollectionsList + handleToggleMobileSidebar={handleToggleMobileSidebar} openCollections={openCollections} onClose={onClose} onOpen={onOpen} diff --git a/frontend/src/metabase/collections/containers/CollectionSidebar/Collections/CollectionsList/CollectionsList.jsx b/frontend/src/metabase/collections/containers/CollectionSidebar/Collections/CollectionsList/CollectionsList.jsx index da6aa0914f1a49d54265429c1c617f61fe80c42c..d47bb87e5d87db9604fa0bc4cd72779e325e1481 100644 --- a/frontend/src/metabase/collections/containers/CollectionSidebar/Collections/CollectionsList/CollectionsList.jsx +++ b/frontend/src/metabase/collections/containers/CollectionSidebar/Collections/CollectionsList/CollectionsList.jsx @@ -24,6 +24,7 @@ function ToggleChildCollectionButton({ action, collectionId, isOpen }) { function handleClick(e) { e.preventDefault(); + e.stopPropagation(); action(collectionId); } @@ -61,6 +62,7 @@ function Collection({ depth, currentCollection, filter, + handleToggleMobileSidebar, initialIcon, onClose, onOpen, @@ -81,7 +83,7 @@ function Collection({ // when we click on a link, if there are children, // expand to show sub collections function handleClick() { - children && action(id); + handleToggleMobileSidebar(); } return ( @@ -110,6 +112,7 @@ function Collection({ {children && isOpen && ( <ChildrenContainer> <CollectionsList + handleToggleMobileSidebar={handleToggleMobileSidebar} openCollections={openCollections} onOpen={onOpen} onClose={onClose} @@ -127,6 +130,7 @@ function Collection({ function CollectionsList({ collections, filter, + handleToggleMobileSidebar, initialIcon, depth = 1, ...otherProps @@ -140,6 +144,7 @@ function CollectionsList({ collection={collection} depth={depth} filter={filter} + handleToggleMobileSidebar={handleToggleMobileSidebar} initialIcon={initialIcon} key={collection.id} {...otherProps} diff --git a/frontend/src/metabase/collections/containers/CollectionSidebar/RootCollectionLink/RootCollectionLink.jsx b/frontend/src/metabase/collections/containers/CollectionSidebar/RootCollectionLink/RootCollectionLink.jsx index 9af0e65bb657a354c78ff4b1681a0be215dac4a2..853ac25b306dd53cdf13a0783e1ee5d4ecdb3257 100644 --- a/frontend/src/metabase/collections/containers/CollectionSidebar/RootCollectionLink/RootCollectionLink.jsx +++ b/frontend/src/metabase/collections/containers/CollectionSidebar/RootCollectionLink/RootCollectionLink.jsx @@ -11,17 +11,27 @@ import CollectionLink from "metabase/collections/components/CollectionLink"; import { Container } from "./RootCollectionLink.styled"; const propTypes = { + handleToggleMobileSidebar: PropTypes.func.isRequired, isRoot: PropTypes.bool.isRequired, root: PropTypes.object.isRequired, }; -export default function CollectionSidebarHeader({ isRoot, root }) { +export default function RootCollectionLink({ + handleToggleMobileSidebar, + isRoot, + root, +}) { + function handleClick() { + handleToggleMobileSidebar(); + } + return ( <Container> <CollectionDropTarget collection={root}> {({ highlighted, hovered }) => ( <CollectionLink to={Urls.collection({ id: "root" })} + onClick={handleClick} selected={isRoot} highlighted={highlighted} hovered={hovered} @@ -35,4 +45,4 @@ export default function CollectionSidebarHeader({ isRoot, root }) { ); } -CollectionSidebarHeader.propTypes = propTypes; +RootCollectionLink.propTypes = propTypes; diff --git a/frontend/test/metabase/scenarios/collections/collections-sidebar.cy.spec.js b/frontend/test/metabase/scenarios/collections/collections-sidebar.cy.spec.js index 19c089abb5b2a922d88625bf8630dc5cc1ff9c2f..be91f880763dbfdc3abe196a80ab4c27f2249d6c 100644 --- a/frontend/test/metabase/scenarios/collections/collections-sidebar.cy.spec.js +++ b/frontend/test/metabase/scenarios/collections/collections-sidebar.cy.spec.js @@ -27,4 +27,17 @@ describe("collections sidebar (metabase#15006)", () => { cy.icon("burger"); }); + + it("should close collections sidebar when collection is clicked in mobile screen size", () => { + cy.viewport(480, 800); + cy.icon("burger").click(); + + sidebar().should("be.visible"); + + sidebar().within(() => { + cy.findByText("First collection").click(); + }); + + cy.icon("burger"); + }); }); diff --git a/frontend/test/metabase/scenarios/collections/collections.cy.spec.js b/frontend/test/metabase/scenarios/collections/collections.cy.spec.js index a0f9bfe95be5e1822d550621e0f92be17dcfdfe6..e88f09faf03f2bd58cc46553744f701dc8c8fdd0 100644 --- a/frontend/test/metabase/scenarios/collections/collections.cy.spec.js +++ b/frontend/test/metabase/scenarios/collections/collections.cy.spec.js @@ -7,6 +7,7 @@ import { openOrdersTable, sidebar, } from "__support__/e2e/cypress"; +import { displaySidebarChildOf } from "./helpers/e2e-collections-sidebar.js"; import { USERS, USER_GROUPS } from "__support__/e2e/cypress_data"; const { nocollection } = USERS; @@ -122,7 +123,7 @@ describe("scenarios > collection_defaults", () => { it("should allow a user to expand a collection without navigating to it", () => { cy.visit("/collection/root"); // 1. click on the chevron to expand the sub collection - openDropdownFor("First collection"); + displaySidebarChildOf("First collection"); // 2. I should see the nested collection name cy.findByText("First collection"); cy.findByText("Second collection"); @@ -153,10 +154,10 @@ describe("scenarios > collection_defaults", () => { }); cy.visit("/collection/root"); // 1. Expand out via the chevrons so that all collections are showing - openDropdownFor("First collection"); - openDropdownFor("Second collection"); - openDropdownFor("Third collection"); - openDropdownFor("Fourth collection"); + displaySidebarChildOf("First collection"); + displaySidebarChildOf("Second collection"); + displaySidebarChildOf("Third collection"); + displaySidebarChildOf("Fourth collection"); // 2. Ensure we can see the entire "Fifth level with a long name" collection text cy.findByText("Fifth collection with a very long name"); }); @@ -324,7 +325,7 @@ describe("scenarios > collection_defaults", () => { cy.visit("/collection/root"); cy.get("[class*=CollectionSidebar]").as("sidebar"); - openDropdownFor("Your personal collection"); + displaySidebarChildOf("Your personal collection"); cy.findByText(COLLECTION); cy.get("@sidebar") .contains("Our analytics") @@ -396,7 +397,7 @@ describe("scenarios > collection_defaults", () => { it("should update UI when nested child collection is moved to the root collection (metabase#14482)", () => { cy.visit("/collection/root"); cy.log("Move 'Second collection' to the root"); - openDropdownFor("First collection"); + displaySidebarChildOf("First collection"); cy.findByText("Second collection").click(); cy.icon("pencil").click(); cy.findByText("Edit this collection").click(); @@ -454,6 +455,7 @@ describe("scenarios > collection_defaults", () => { it("collections without sub-collections shouldn't have chevron icon (metabase#14753)", () => { cy.visit("/collection/root"); + sidebar() .findByText("Your personal collection") .parent() @@ -461,9 +463,8 @@ describe("scenarios > collection_defaults", () => { .should("not.exist"); // Ensure if sub-collection is archived, the chevron is not displayed + displaySidebarChildOf("First collection"); sidebar() - .findByText("First collection") - .click() .findByText("Second collection") .click(); cy.icon("pencil").click(); @@ -611,14 +612,6 @@ function createPulse() { cy.findByText("Create pulse").click(); } -function openDropdownFor(collectionName) { - cy.findByText(collectionName) - .parent() - .find(".Icon-chevronright") - .eq(0) // there may be more nested icons, but we need the top level one - .click(); -} - function openEllipsisMenuFor(item) { cy.findByText(item) .closest("tr") diff --git a/frontend/test/metabase/scenarios/collections/helpers/e2e-collections-sidebar.js b/frontend/test/metabase/scenarios/collections/helpers/e2e-collections-sidebar.js new file mode 100644 index 0000000000000000000000000000000000000000..8aaec80f08afa52dedacc9234a9212a224a0f849 --- /dev/null +++ b/frontend/test/metabase/scenarios/collections/helpers/e2e-collections-sidebar.js @@ -0,0 +1,7 @@ +export function displaySidebarChildOf(collectionName) { + cy.findByText(collectionName) + .parent() + .find(".Icon-chevronright") + .eq(0) // there may be more nested icons, but we need the top level one + .click(); +} diff --git a/frontend/test/metabase/scenarios/collections/permissions.cy.spec.js b/frontend/test/metabase/scenarios/collections/permissions.cy.spec.js index 9319bde23829090d64c4369bd7a41d99ff836ff4..b77f8f9d4a4aa90ff8c50e28bff0c226ffd327e2 100644 --- a/frontend/test/metabase/scenarios/collections/permissions.cy.spec.js +++ b/frontend/test/metabase/scenarios/collections/permissions.cy.spec.js @@ -17,6 +17,7 @@ import { sidebar, openNativeEditor, } from "__support__/e2e/cypress"; +import { displaySidebarChildOf } from "./helpers/e2e-collections-sidebar.js"; import { USERS } from "__support__/e2e/cypress_data"; const PERMISSIONS = { @@ -45,8 +46,8 @@ describe("collection permissions", () => { it("should offer to save dashboard to a currently opened collection", () => { cy.visit("/collection/root"); sidebar().within(() => { - cy.findByText("First collection").click(); - cy.findByText("Second collection").click(); + displaySidebarChildOf("First collection"); + displaySidebarChildOf("Second collection"); }); cy.icon("add").click(); cy.findByText("New dashboard").click();