Skip to content

Commit

Permalink
Squashed branch
Browse files Browse the repository at this point in the history
  • Loading branch information
rafpaf committed Apr 29, 2024
1 parent e2ebff4 commit 57d7ec3
Show file tree
Hide file tree
Showing 12 changed files with 172 additions and 74 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ describe("scenarios > admin > databases > sample database", () => {

cy.wait("@loadDatabases");
cy.findByTestId("main-navbar-root").within(() => {
cy.findByText("Browse data").should("not.exist");
cy.findByLabelText("Browse data").should("not.exist");
});

cy.visit("/admin/databases");
Expand All @@ -293,8 +293,8 @@ describe("scenarios > admin > databases > sample database", () => {

cy.wait("@loadDatabases");
cy.findByTestId("main-navbar-root").within(() => {
cy.findByText("Browse data").should("exist");
cy.findByText("Browse data").click();
cy.findByLabelText("Browse data").should("exist");
cy.findByLabelText("Browse data").click();
});

cy.findByRole("tab", { name: "Databases" }).click();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ describe("11914, 18978, 18977", () => {
it("should not display query editing controls and 'Browse Data' link", () => {
cy.log("Make sure we don't prompt user to browse data from the sidebar");
openNavigationSidebar();
cy.findByTestId("main-navbar-root").should("not.contain", "Browse data");
cy.findByLabelText("Browse data").should("not.exist");

cy.log("Make sure we don't prompt user to create a new query");
appBar().icon("add").click();
Expand Down
2 changes: 2 additions & 0 deletions frontend/src/metabase-types/api/mocks/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,8 @@ export const createMockSettings = (
"enable-enhancements?": false,
"enable-nested-queries": true,
"enable-query-caching": undefined,
"expand-browse-in-nav": true,
"expand-bookmarks-in-nav": true,
"query-caching-ttl-ratio": 10,
"query-caching-min-ttl": 60,
"enable-password-login": true,
Expand Down
10 changes: 8 additions & 2 deletions frontend/src/metabase-types/api/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -311,13 +311,19 @@ interface PublicSettings {
"version-info-last-checked": string | null;
}

export interface UserSettings {
export interface BrowseFilterSettings {
"browse-filter-only-verified-models"?: boolean;
}

export type UserSettings = {
"dismissed-browse-models-banner"?: boolean;
"dismissed-custom-dashboard-toast"?: boolean;
"last-used-native-database-id"?: number | null;
"notebook-native-preview-shown"?: boolean;
"notebook-native-preview-sidebar-width"?: number | null;
}
"expand-browse-in-nav"?: boolean;
"expand-bookmarks-in-nav"?: boolean;
} & BrowseFilterSettings;

export type Settings = InstanceSettings &
PublicSettings &
Expand Down
18 changes: 16 additions & 2 deletions frontend/src/metabase/common/hooks/use-setting/use-setting.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,23 @@
import { useSelector } from "metabase/lib/redux";
import { useDispatch, useSelector } from "metabase/lib/redux";
import type { UpdateUserSettingOptions } from "metabase/redux/settings";
import { updateUserSetting } from "metabase/redux/settings";
import { getSetting } from "metabase/selectors/settings";
import type { Settings } from "metabase-types/api";
import type { Settings, UserSettings } from "metabase-types/api";

export const useSetting = <SettingName extends keyof Settings>(
settingName: SettingName,
) => {
return useSelector(state => getSetting(state, settingName));
};

export const useUserSetting = <T extends keyof UserSettings>(
key: T,
options?: UpdateUserSettingOptions,
): [UserSettings[T], (value: UserSettings[T]) => void] => {
const currentValue = useSetting(key);
const dispatch = useDispatch();
const setter = (value: UserSettings[T]) => {
dispatch(updateUserSetting({ key, value }, options));
};
return [currentValue, setter];
};
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { SidebarLink } from "./SidebarItems";
const openSidebarCSS = css`
width: ${NAV_SIDEBAR_WIDTH};
border-right: 1px solid ${color("border")};
border-inline-end: 1px solid ${color("border")};
${breakpointMaxSmall} {
width: 90vw;
Expand Down Expand Up @@ -44,7 +44,7 @@ export const Sidebar = styled.aside<{ isOpen: boolean }>`
${breakpointMaxSmall} {
position: absolute;
top: 0;
left: 0;
inline-start: 0;
}
`;

Expand Down Expand Up @@ -85,8 +85,8 @@ export const SidebarContentRoot = styled.div`
export const SidebarSection = styled.div`
margin-top: ${space(1)};
margin-bottom: ${space(2)};
padding-left: ${space(2)};
padding-right: ${space(2)};
padding-inline-start: ${space(2)};
padding-inline-end: ${space(2)};
`;

export const SidebarHeadingWrapper = styled.div`
Expand All @@ -101,12 +101,12 @@ export const SidebarHeading = styled.h4`
font-size: 11px;
text-transform: uppercase;
letter-spacing: 0.45px;
padding-left: ${space(2)};
padding-inline-start: ${space(2)};
`;

export const CollectionsMoreIconContainer = styled.button`
margin-left: auto;
margin-right: ${space(1)};
margin-inline-start: auto;
margin-inline-end: ${space(1)};
cursor: pointer;
`;

Expand Down Expand Up @@ -140,7 +140,7 @@ export const LoadingAndErrorTitle = styled.h2`
`;

export const PaddedSidebarLink = styled(SidebarLink)`
padding-left: 12px;
padding-inline-start: 12px;
`;

export const AddYourOwnDataLink = styled(SidebarLink)`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
renderWithProviders,
screen,
waitForLoaderToBeRemoved,
within,
} from "__support__/ui";
import { ROOT_COLLECTION } from "metabase/entities/collections";
import * as Urls from "metabase/lib/urls";
Expand Down Expand Up @@ -191,28 +192,29 @@ describe("nav > containers > MainNavbar", () => {
describe("browse data link", () => {
it("should render", async () => {
await setup();
const link = screen.getByRole("link", { name: /Browse data/i });
const listItem = screen.getByRole("listitem", { name: /Browse data/i });
const link = within(listItem).getByRole("link");
expect(link).toBeInTheDocument();
expect(link).toHaveAttribute("href", "/browse");
expect(link).toHaveAttribute("href", "/browse/databases");
});

it("should not render when a user has no data access", async () => {
await setup({ hasDataAccess: false });
expect(
screen.queryByRole("link", { name: /Browse data/i }),
screen.queryByRole("listitem", { name: /Browse data/i }),
).not.toBeInTheDocument();
});

it("should be highlighted if selected", async () => {
await setup({ pathname: "/browse" });
const link = screen.getByRole("listitem", { name: /Browse data/i });
expect(link).toHaveAttribute("aria-selected", "true");
await setup({ pathname: "/browse/databases" });
const listItem = screen.getByRole("listitem", { name: /Browse data/i });
expect(listItem).toHaveAttribute("aria-selected", "true");
});

it("should be highlighted if child route selected", async () => {
await setup({ pathname: "/browse/databases/1" });
const link = screen.getByRole("listitem", { name: /Browse data/i });
expect(link).toHaveAttribute("aria-selected", "true");
const listItem = screen.getByRole("listitem", { name: /Browse data/i });
expect(listItem).toHaveAttribute("aria-selected", "true");
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ interface CollectionSidebarBookmarksProps {
newIndex: number;
oldIndex: number;
}) => void;
onToggle: (isExpanded: boolean) => void;
initialState: "expanded" | "collapsed";
}

interface BookmarkItemProps {
Expand All @@ -56,9 +58,6 @@ interface BookmarkItemProps {
onDeleteBookmark: (bookmark: Bookmark) => void;
}

const BOOKMARKS_INITIALLY_VISIBLE =
localStorage.getItem("shouldDisplayBookmarks") !== "false";

function isBookmarkSelected(bookmark: Bookmark, selectedItem?: SelectedItem) {
if (!selectedItem) {
return false;
Expand Down Expand Up @@ -116,6 +115,8 @@ const BookmarkList = ({
onSelect,
onDeleteBookmark,
reorderBookmarks,
onToggle,
initialState,
}: CollectionSidebarBookmarksProps) => {
const [orderedBookmarks, setOrderedBookmarks] = useState(bookmarks);
const [isSorting, setIsSorting] = useState(false);
Expand All @@ -128,10 +129,6 @@ const BookmarkList = ({
activationConstraint: { distance: 15 },
});

const onToggleBookmarks = useCallback(isVisible => {
localStorage.setItem("shouldDisplayBookmarks", String(isVisible));
}, []);

const handleSortStart = useCallback(() => {
document.body.classList.add(GrabberS.grabbing);
setIsSorting(true);
Expand All @@ -153,11 +150,11 @@ const BookmarkList = ({
return (
<CollapseSection
header={<SidebarHeading>{t`Bookmarks`}</SidebarHeading>}
initialState={BOOKMARKS_INITIALLY_VISIBLE ? "expanded" : "collapsed"}
initialState={initialState}
iconPosition="right"
iconSize={8}
headerClass={CS.mb1}
onToggle={onToggleBookmarks}
onToggle={onToggle}
>
<DndContext
onDragEnd={handleSortEnd}
Expand Down

0 comments on commit 57d7ec3

Please sign in to comment.