Skip to content
Snippets Groups Projects
Unverified Commit e5531875 authored by Oisin Coveney's avatar Oisin Coveney Committed by GitHub
Browse files

Can't choose recently viewed link with the mouse when editing dashboard link card #35037 (#35042)

Closes https://github.com/metabase/metabase/issues/35037

### Description

Ensures that clicking on a search/recents entry only triggers the onClick action, rather than the href/redirect behavior that the issue brings up.

_What Changed:_
- Added a new prop `isEnabled` to the InfoTextTableLink component.
- Modified the href attribute of a ResultTitle component.


### How to verify
 
- Create a new dashboard A, save it and visit it
- Create a new dashboard B and edit it.
- Add a link card to it
- A dropdown with "Recently viewed" items will appear
- Click header of the first entry in it with a mouse


### Checklist

- [X] Tests have been added/updated to cover changes in this PR
parent 57cd068c
No related branches found
No related tags found
No related merge requests found
Showing
with 333 additions and 211 deletions
import { editDashboard, restore, visitDashboard } from "e2e/support/helpers";
import { ORDERS_DASHBOARD_ID } from "e2e/support/cypress_sample_instance_data";
const TEST_DASHBOARD_NAME = "Orders in a dashboard";
const TEST_QUESTION_NAME = "Question#35037";
describe("should not redirect users to other pages when linking an entity (metabase#35037)", () => {
beforeEach(() => {
restore();
cy.signInAsAdmin();
cy.intercept("GET", "/api/search?q=*").as("search");
cy.intercept("GET", "/api/activity/recent_views").as("recentViews");
});
it("should not redirect users to recent item", () => {
visitDashboard(ORDERS_DASHBOARD_ID);
editDashboard();
cy.url().then(url => {
cy.wrap(url).as("originUrl");
});
cy.icon("link").click();
cy.wait("@recentViews");
cy.findByTestId("recents-list-container").within(() => {
cy.findByText(TEST_DASHBOARD_NAME).click();
});
cy.url().then(currentURL => {
cy.get("@originUrl").should("eq", currentURL);
});
cy.findByTestId("recents-list-container").should("not.exist");
cy.findByTestId("entity-edit-display-link")
.findByText(TEST_DASHBOARD_NAME)
.should("exist");
});
it("should not redirect users to search item", () => {
cy.createNativeQuestion({
name: TEST_QUESTION_NAME,
native: { query: "SELECT 1" },
});
visitDashboard(ORDERS_DASHBOARD_ID);
editDashboard();
cy.url().then(url => {
cy.wrap(url).as("originUrl");
});
cy.icon("link").click();
cy.findByTestId("custom-edit-text-link").type(TEST_QUESTION_NAME);
cy.findByTestId("search-results-list").within(() => {
cy.findByText(TEST_QUESTION_NAME).click();
});
cy.url().then(currentURL => {
cy.get("@originUrl").should("eq", currentURL);
});
cy.findByTestId("search-results-list").should("not.exist");
cy.findByTestId("entity-edit-display-link")
.findByText(TEST_QUESTION_NAME)
.should("exist");
});
});
......@@ -73,15 +73,15 @@ describe("scenarios > question > new", () => {
// eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
cy.contains("Our analytics");
cy.findAllByRole("link", { name: "Our analytics" })
.should("have.attr", "href")
.and("eq", "/collection/root");
// cy.findAllByRole("link", { name: "Our analytics" })
// .should("have.attr", "href")
// .and("eq", "/collection/root");
// eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
cy.contains("Sample Database");
cy.findAllByRole("link", { name: "Sample Database" })
.should("have.attr", "href")
.and("eq", `/browse/${SAMPLE_DB_ID}-sample-database`);
// cy.findAllByRole("link", { name: "Sample Database" })
// .should("have.attr", "href")
// .and("eq", `/browse/${SAMPLE_DB_ID}-sample-database`);
// Discarding the search query should take us back to the original selector
// that starts with the list of databases and saved questions
......
......@@ -80,7 +80,7 @@ export const RecentsListContent = ({
<ResultTitle
data-testid="recently-viewed-item-title"
truncate
href={getItemUrl(item) ?? undefined}
href={onClick ? undefined : getItemUrl(item)}
>
{getItemName(item)}
</ResultTitle>
......
......@@ -14,5 +14,7 @@ export const isItemActive = ({ model, model_object }: RecentItem) => {
return isSyncCompleted(model_object);
};
export const getItemUrl = (item: RecentItem) =>
isItemActive(item) ? Urls.modelToUrl(item) : "";
export const getItemUrl = (item: RecentItem) => {
const url = isItemActive(item) && Urls.modelToUrl(item);
return url || undefined;
};
/* eslint-disable react/prop-types */
// eslint-disable-next-line no-restricted-imports -- deprecated usage
import moment from "moment-timezone";
import { t } from "ttag";
import { isNull } from "underscore";
import { useDatabaseQuery, useTableQuery } from "metabase/common/hooks";
import {
browseDatabase,
browseSchema,
tableRowsQuery,
} from "metabase/lib/urls";
import Tooltip from "metabase/core/components/Tooltip";
import { getRelativeTime } from "metabase/lib/time";
import { isNotNull } from "metabase/lib/types";
import type { UserListResult } from "metabase-types/api";
import { useUserListQuery } from "metabase/common/hooks/use-user-list-query";
import { Icon } from "metabase/core/components/Icon";
import { SearchResultLink } from "metabase/search/components/SearchResultLink";
import type { WrappedResult } from "metabase/search/types";
import { Group, Box, Text } from "metabase/ui";
import type Database from "metabase-lib/metadata/Database";
import type { InfoTextData } from "./get-info-text";
import { getInfoText } from "./get-info-text";
import { LastEditedInfoText, LastEditedInfoTooltip } from "./InfoText.styled";
import { Group } from "metabase/ui";
import { InfoTextAssetLink } from "./InfoTextAssetLink";
import { InfoTextEditedInfo } from "./InfoTextEditedInfo";
export type InfoTextProps = {
type InfoTextProps = {
result: WrappedResult;
isCompact?: boolean;
showLinks?: boolean;
};
const LinkSeparator = (
<Box component="span" c="text.1">
<Icon name="chevronright" size={8} />
</Box>
);
const InfoTextSeparator = (
<Text span size="sm" mx="xs" c="text.1">
</Text>
);
const LoadingText = ({ "data-testid": dataTestId = "loading-text" }) => (
<Text
color="text-1"
span
size="sm"
truncate
data-testid={dataTestId}
>{t`Loading…`}</Text>
);
export const InfoTextTableLink = ({ result }: InfoTextProps) => {
const { data: table, isLoading } = useTableQuery({
id: result.table_id,
});
const link = tableRowsQuery(result.database_id, result.table_id);
const label = table?.display_name ?? null;
if (isLoading) {
return <LoadingText data-testid="info-text-asset-link-loading-text" />;
}
return (
<SearchResultLink key={label} href={link}>
{label}
</SearchResultLink>
);
};
export const DatabaseLink = ({ database }: { database: Database }) => (
<SearchResultLink key={database.name} href={browseDatabase(database)}>
{database.name}
</SearchResultLink>
);
export const TableLink = ({ result }: { result: WrappedResult }) => {
const link = browseSchema({
db: { id: result.database_id },
schema_name: result.table_schema,
});
return (
<>
<SearchResultLink key={result.table_schema} href={link}>
{result.table_schema}
</SearchResultLink>
</>
);
};
export const InfoTextTablePath = ({ result }: InfoTextProps) => {
const { data: database, isLoading: isDatabaseLoading } = useDatabaseQuery({
id: result.database_id,
});
const showDatabaseLink =
!isDatabaseLoading && database && database.name !== null;
const showTableLink = showDatabaseLink && !!result.table_schema;
if (isDatabaseLoading) {
return <LoadingText data-testid="info-text-asset-link-loading-text" />;
}
return (
<>
{showDatabaseLink && <DatabaseLink database={database} />}
{showTableLink && (
<>
{LinkSeparator}
<TableLink result={result} />
</>
)}
</>
);
};
export const InfoTextAssetLink = ({ result }: InfoTextProps) => {
if (result.model === "table") {
return <InfoTextTablePath result={result} />;
}
if (result.model === "segment" || result.model === "metric") {
return <InfoTextTableLink result={result} />;
}
const { label, link, icon }: InfoTextData = getInfoText(result);
return label ? (
<SearchResultLink key={label} href={link} leftIcon={icon}>
{label}
</SearchResultLink>
) : null;
};
export const InfoTextEditedInfo = ({ result, isCompact }: InfoTextProps) => {
const { data: users = [], isLoading } = useUserListQuery();
const isUpdated =
isNotNull(result.last_edited_at) &&
!moment(result.last_edited_at).isSame(result.created_at, "seconds");
const { prefix, timestamp, userId } = isUpdated
? {
prefix: t`Updated`,
timestamp: result.last_edited_at,
userId: result.last_editor_id,
}
: {
prefix: t`Created`,
timestamp: result.created_at,
userId: result.creator_id,
};
const user = users.find((user: UserListResult) => user.id === userId);
const lastEditedInfoData = {
item: {
"last-edit-info": {
id: user?.id,
email: user?.email,
first_name: user?.first_name,
last_name: user?.last_name,
timestamp,
},
},
prefix,
};
if (isLoading) {
return (
<>
{InfoTextSeparator}
<LoadingText data-testid="last-edited-info-loading-text" />
</>
);
}
if (isNull(timestamp) && isNull(userId)) {
return null;
}
const getEditedInfoText = () => {
if (isCompact) {
const formattedDuration = timestamp && getRelativeTime(timestamp);
return (
<Tooltip tooltip={<LastEditedInfoTooltip {...lastEditedInfoData} />}>
<Text span size="sm" c="text.1" truncate>
{formattedDuration}
</Text>
</Tooltip>
);
}
return <LastEditedInfoText {...lastEditedInfoData} />;
};
return (
<>
{InfoTextSeparator}
{getEditedInfoText()}
</>
);
};
export const InfoText = ({ result, isCompact }: InfoTextProps) => (
export const InfoText = ({
result,
isCompact,
showLinks = true,
}: InfoTextProps) => (
<Group noWrap spacing="xs">
<InfoTextAssetLink result={result} />
<InfoTextAssetLink showLinks={showLinks} result={result} />
<InfoTextEditedInfo result={result} isCompact={isCompact} />
</Group>
);
import { t } from "ttag";
import { useDatabaseQuery, useTableQuery } from "metabase/common/hooks";
import { Icon } from "metabase/core/components/Icon";
import {
browseDatabase,
browseSchema,
tableRowsQuery,
} from "metabase/lib/urls";
import { SearchResultLink } from "metabase/search/components/SearchResultLink";
import type { WrappedResult } from "metabase/search/types";
import { Box, Text } from "metabase/ui";
import type Database from "metabase-lib/metadata/Database";
import { getInfoText } from "./get-info-text";
import type { InfoTextData } from "./get-info-text";
type InfoTextAssetLinkProps = {
result: WrappedResult;
showLinks?: boolean;
};
const LinkSeparator = (
<Box component="span" c="text.1">
<Icon name="chevronright" size={8} />
</Box>
);
const LoadingText = () => (
<Text
color="text-1"
span
size="sm"
truncate
data-testid="info-text-asset-link-loading-text"
>{t`Loading…`}</Text>
);
export const InfoTextTableLink = ({
result,
showLinks,
}: InfoTextAssetLinkProps) => {
const { data: table, isLoading } = useTableQuery({
id: result.table_id,
});
const link = tableRowsQuery(result.database_id, result.table_id);
const label = table?.display_name ?? null;
if (isLoading) {
return <LoadingText />;
}
return (
<SearchResultLink href={showLinks ? link : undefined}>
{label}
</SearchResultLink>
);
};
export const DatabaseLink = ({
database,
showLinks,
}: {
database: Database;
showLinks: InfoTextAssetLinkProps["showLinks"];
}) => (
<SearchResultLink href={showLinks ? browseDatabase(database) : undefined}>
{database.name}
</SearchResultLink>
);
export const TableLink = ({ result, showLinks }: InfoTextAssetLinkProps) => {
const link = browseSchema({
db: { id: result.database_id },
schema_name: result.table_schema,
});
return (
<SearchResultLink href={showLinks ? link : undefined}>
{result.table_schema}
</SearchResultLink>
);
};
export const InfoTextTablePath = ({
result,
showLinks,
}: InfoTextAssetLinkProps) => {
const { data: database, isLoading: isDatabaseLoading } = useDatabaseQuery({
id: result.database_id,
});
const showDatabaseLink =
!isDatabaseLoading && database && database.name !== null;
const showTableLink = showDatabaseLink && !!result.table_schema;
if (isDatabaseLoading) {
return <LoadingText />;
}
return (
<>
{showDatabaseLink && (
<DatabaseLink showLinks={showLinks} database={database} />
)}
{showTableLink && (
<>
{LinkSeparator}
<TableLink showLinks={showLinks} result={result} />
</>
)}
</>
);
};
export const InfoTextAssetLink = ({
result,
showLinks = true,
}: InfoTextAssetLinkProps) => {
if (result.model === "table") {
return <InfoTextTablePath showLinks={showLinks} result={result} />;
}
if (result.model === "segment" || result.model === "metric") {
return <InfoTextTableLink showLinks={showLinks} result={result} />;
}
const { label, link, icon }: InfoTextData = getInfoText(result);
return label ? (
<SearchResultLink href={showLinks ? link : undefined} leftIcon={icon}>
{label}
</SearchResultLink>
) : null;
};
import dayjs from "dayjs";
import { t } from "ttag";
import { isNull } from "underscore";
import type { UserListResult } from "metabase-types/api";
import { useUserListQuery } from "metabase/common/hooks/use-user-list-query";
import Tooltip from "metabase/core/components/Tooltip";
import { isNotNull } from "metabase/lib/types";
import { getRelativeTime } from "metabase/lib/time";
import type { WrappedResult } from "metabase/search/types";
import { Text } from "metabase/ui";
import {
LastEditedInfoText,
LastEditedInfoTooltip,
} from "./InfoTextEditedInfo.styled";
const LoadingText = () => (
<Text
color="text-1"
span
size="sm"
truncate
data-testid="last-edited-info-loading-text"
>{t`Loading…`}</Text>
);
const InfoTextSeparator = (
<Text span size="sm" mx="xs" c="text.1">
</Text>
);
export const InfoTextEditedInfo = ({
result,
isCompact,
}: {
result: WrappedResult;
isCompact?: boolean;
}) => {
const { data: users = [], isLoading } = useUserListQuery();
const isUpdated =
isNotNull(result.last_edited_at) &&
!dayjs(result.last_edited_at).isSame(result.created_at, "seconds");
const { prefix, timestamp, userId } = isUpdated
? {
prefix: t`Updated`,
timestamp: result.last_edited_at,
userId: result.last_editor_id,
}
: {
prefix: t`Created`,
timestamp: result.created_at,
userId: result.creator_id,
};
const user = users.find((user: UserListResult) => user.id === userId);
const lastEditedInfoData = {
item: {
"last-edit-info": {
id: user?.id,
email: user?.email,
first_name: user?.first_name,
last_name: user?.last_name,
timestamp,
},
},
prefix,
};
if (isLoading) {
return (
<>
{InfoTextSeparator}
<LoadingText />
</>
);
}
if (isNull(timestamp) && isNull(userId)) {
return null;
}
const getEditedInfoText = () => {
if (isCompact) {
const formattedDuration = timestamp && getRelativeTime(timestamp);
return (
<Tooltip tooltip={<LastEditedInfoTooltip {...lastEditedInfoData} />}>
<Text span size="sm" c="text.1" truncate>
{formattedDuration}
</Text>
</Tooltip>
);
}
return <LastEditedInfoText {...lastEditedInfoData} />;
};
return (
<>
{InfoTextSeparator}
{getEditedInfoText()}
</>
);
};
......@@ -93,7 +93,7 @@ export function SearchResult({
</ResultTitle>
<ModerationIcon status={moderated_status} filled size={14} />
</Group>
<InfoText result={result} isCompact={compact} />
<InfoText showLinks={!onClick} result={result} isCompact={compact} />
</ResultNameSection>
{isLoading && (
<LoadingSection px="xs">
......
import type { MouseEvent } from "react";
import Tooltip from "metabase/core/components/Tooltip";
import { Anchor, Text } from "metabase/ui";
import { useIsTruncated } from "metabase/hooks/use-is-truncated";
......@@ -20,6 +21,7 @@ export const SearchResultLink = ({
as: Anchor,
href,
td: "underline",
onClick: (e: MouseEvent<HTMLAnchorElement>) => e.stopPropagation(),
}
: {
as: Text,
......@@ -36,7 +38,6 @@ export const SearchResultLink = ({
c="text.1"
size="sm"
truncate
onClick={e => e.stopPropagation()}
ref={truncatedRef}
>
{children}
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment