fix: Enhance URL handling in table by rendering URL column types with <a> tag. (#37179)
## Description <ins>Problem</ins> URLs in table were not being rendered as links, resulting in inconsistent user experience(missing context menus. <ins>Root cause</ins> URLs were rendered in `<div>` instead of `<a>`, making the component lack links related features.. <ins>Solution</ins> This PR handles... - Rendering URLs as links in BasicCell for a better user experience. - Adding specific types for column properties for more robust data validation and type checking. - Adding unit tests for BasicCell functionality to ensure accurate rendering and behavior. - Simplifies the AutoToolTipComponent by removing unncessary `LinkWrapper` component Fixes #24769 _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="@tag.Table" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/11681339029> > Commit: b7c5d176b35407923a120bb19e40252e3a61b628 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=11681339029&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Table` > Spec: > <hr>Tue, 05 Nov 2024 10:23:38 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes - **New Features** - Enhanced type safety for `compactMode` and `columnType` properties across various components. - Improved rendering logic in the `AutoToolTipComponent` for better control based on `columnType`. - Optimized rendering in the `BasicCell` component using `useMemo`. - **Bug Fixes** - Resolved inconsistencies in type definitions for `BasicCell`, `PlainTextCell`, and `SelectCell` components. - Updated tooltip behavior in the `AutoToolTipComponent` to ensure accurate rendering. - **Tests** - Introduced a new test suite for the `BasicCell` component, ensuring proper rendering and interaction behaviors. - Refined test cases for the `AutoToolTipComponent` to verify accurate rendering under various conditions. - Updated test case for URL column verification to check attributes directly instead of navigation. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
parent
4e18827512
commit
0288f5b9ef
|
|
@ -20,18 +20,23 @@ describe(
|
||||||
table.ReadTableRowColumnData(3, 0, "v2").then(($cellData) => {
|
table.ReadTableRowColumnData(3, 0, "v2").then(($cellData) => {
|
||||||
expect($cellData).to.eq("Profile pic");
|
expect($cellData).to.eq("Profile pic");
|
||||||
});
|
});
|
||||||
table.AssertURLColumnNavigation(
|
|
||||||
0,
|
agHelper
|
||||||
0,
|
.GetElement(`${table._tableRowColumnData(0, 0, "v2")} a`)
|
||||||
"https://randomuser.me/api/portraits/med/women/39.jpg",
|
.should(
|
||||||
"v2",
|
"have.attr",
|
||||||
);
|
"href",
|
||||||
table.AssertURLColumnNavigation(
|
"https://randomuser.me/api/portraits/med/women/39.jpg",
|
||||||
3,
|
)
|
||||||
0,
|
.should("have.attr", "target", "_blank");
|
||||||
"https://randomuser.me/api/portraits/med/men/52.jpg",
|
agHelper
|
||||||
"v2",
|
.GetElement(`${table._tableRowColumnData(3, 0, "v2")} a`)
|
||||||
);
|
.should(
|
||||||
|
"have.attr",
|
||||||
|
"href",
|
||||||
|
"https://randomuser.me/api/portraits/med/men/52.jpg",
|
||||||
|
)
|
||||||
|
.should("have.attr", "target", "_blank");
|
||||||
});
|
});
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
|
|
|
||||||
|
|
@ -546,7 +546,7 @@ export enum IMAGE_VERTICAL_ALIGN {
|
||||||
}
|
}
|
||||||
|
|
||||||
export interface BaseCellComponentProps {
|
export interface BaseCellComponentProps {
|
||||||
compactMode: string;
|
compactMode: CompactMode;
|
||||||
isHidden: boolean;
|
isHidden: boolean;
|
||||||
allowCellWrapping?: boolean;
|
allowCellWrapping?: boolean;
|
||||||
horizontalAlignment?: CellAlignment;
|
horizontalAlignment?: CellAlignment;
|
||||||
|
|
|
||||||
|
|
@ -137,7 +137,7 @@ interface Props {
|
||||||
children: React.ReactNode;
|
children: React.ReactNode;
|
||||||
title: string;
|
title: string;
|
||||||
tableWidth?: number;
|
tableWidth?: number;
|
||||||
columnType?: string;
|
columnType?: ColumnTypes;
|
||||||
className?: string;
|
className?: string;
|
||||||
compactMode?: string;
|
compactMode?: string;
|
||||||
allowCellWrapping?: boolean;
|
allowCellWrapping?: boolean;
|
||||||
|
|
@ -152,38 +152,6 @@ interface Props {
|
||||||
isCellDisabled?: boolean;
|
isCellDisabled?: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
function LinkWrapper(props: Props) {
|
|
||||||
const content = useToolTip(props.children, props.title);
|
|
||||||
|
|
||||||
return (
|
|
||||||
<CellWrapper
|
|
||||||
allowCellWrapping={props.allowCellWrapping}
|
|
||||||
cellBackground={props.cellBackground}
|
|
||||||
className="cell-wrapper"
|
|
||||||
compactMode={props.compactMode}
|
|
||||||
fontStyle={props.fontStyle}
|
|
||||||
horizontalAlignment={props.horizontalAlignment}
|
|
||||||
isCellDisabled={props.isCellDisabled}
|
|
||||||
isCellVisible={props.isCellVisible}
|
|
||||||
isHidden={props.isHidden}
|
|
||||||
isHyperLink
|
|
||||||
isTextType
|
|
||||||
onClick={(e: React.MouseEvent<HTMLDivElement>) => {
|
|
||||||
e.stopPropagation();
|
|
||||||
window.open(props.url, "_blank");
|
|
||||||
}}
|
|
||||||
textColor={props.textColor}
|
|
||||||
textSize={props.textSize}
|
|
||||||
verticalAlignment={props.verticalAlignment}
|
|
||||||
>
|
|
||||||
<div className="link-text">{content}</div>
|
|
||||||
<OpenNewTabIconWrapper className="hidden-icon">
|
|
||||||
<OpenNewTabIcon />
|
|
||||||
</OpenNewTabIconWrapper>
|
|
||||||
</CellWrapper>
|
|
||||||
);
|
|
||||||
}
|
|
||||||
|
|
||||||
export function AutoToolTipComponent(props: Props) {
|
export function AutoToolTipComponent(props: Props) {
|
||||||
const content = useToolTip(
|
const content = useToolTip(
|
||||||
props.children,
|
props.children,
|
||||||
|
|
@ -191,12 +159,27 @@ export function AutoToolTipComponent(props: Props) {
|
||||||
props.columnType === ColumnTypes.BUTTON,
|
props.columnType === ColumnTypes.BUTTON,
|
||||||
);
|
);
|
||||||
|
|
||||||
if (props.columnType === ColumnTypes.URL && props.title) {
|
let contentToRender;
|
||||||
return <LinkWrapper {...props} />;
|
|
||||||
}
|
|
||||||
|
|
||||||
if (props.columnType === ColumnTypes.BUTTON && props.title) {
|
switch (props.columnType) {
|
||||||
return content;
|
case ColumnTypes.BUTTON:
|
||||||
|
if (props.title) {
|
||||||
|
return content;
|
||||||
|
}
|
||||||
|
|
||||||
|
break;
|
||||||
|
case ColumnTypes.URL:
|
||||||
|
contentToRender = (
|
||||||
|
<>
|
||||||
|
<div className="link-text">{content}</div>
|
||||||
|
<OpenNewTabIconWrapper className="hidden-icon">
|
||||||
|
<OpenNewTabIcon />
|
||||||
|
</OpenNewTabIconWrapper>
|
||||||
|
</>
|
||||||
|
);
|
||||||
|
break;
|
||||||
|
default:
|
||||||
|
contentToRender = content;
|
||||||
}
|
}
|
||||||
|
|
||||||
return (
|
return (
|
||||||
|
|
@ -212,12 +195,13 @@ export function AutoToolTipComponent(props: Props) {
|
||||||
isCellDisabled={props.isCellDisabled}
|
isCellDisabled={props.isCellDisabled}
|
||||||
isCellVisible={props.isCellVisible}
|
isCellVisible={props.isCellVisible}
|
||||||
isHidden={props.isHidden}
|
isHidden={props.isHidden}
|
||||||
|
isHyperLink={props.columnType === ColumnTypes.URL}
|
||||||
isTextType
|
isTextType
|
||||||
textColor={props.textColor}
|
textColor={props.textColor}
|
||||||
textSize={props.textSize}
|
textSize={props.textSize}
|
||||||
verticalAlignment={props.verticalAlignment}
|
verticalAlignment={props.verticalAlignment}
|
||||||
>
|
>
|
||||||
{content}
|
{contentToRender}
|
||||||
</CellWrapper>
|
</CellWrapper>
|
||||||
</ColumnWrapper>
|
</ColumnWrapper>
|
||||||
);
|
);
|
||||||
|
|
|
||||||
|
|
@ -54,7 +54,7 @@ test("does not show tooltip for non-button types", () => {
|
||||||
|
|
||||||
test("handles empty tooltip", () => {
|
test("handles empty tooltip", () => {
|
||||||
const { getByText } = render(
|
const { getByText } = render(
|
||||||
<AutoToolTipComponent columnType={ColumnTypes.BUTTON} title="">
|
<AutoToolTipComponent columnType={ColumnTypes.BUTTON} title="Empty button">
|
||||||
<button>Empty button</button>
|
<button>Empty button</button>
|
||||||
</AutoToolTipComponent>,
|
</AutoToolTipComponent>,
|
||||||
);
|
);
|
||||||
|
|
@ -86,25 +86,6 @@ test("does not show tooltip for non-truncated text", () => {
|
||||||
expect(screen.queryByRole("dialog")).not.toBeInTheDocument();
|
expect(screen.queryByRole("dialog")).not.toBeInTheDocument();
|
||||||
});
|
});
|
||||||
|
|
||||||
test("opens a new tab for URL column type when clicked", () => {
|
|
||||||
const openSpy = jest.spyOn(window, "open").mockImplementation(() => null);
|
|
||||||
|
|
||||||
render(
|
|
||||||
<AutoToolTipComponent
|
|
||||||
columnType={ColumnTypes.URL}
|
|
||||||
title="Go to Google"
|
|
||||||
url="https://www.google.com"
|
|
||||||
>
|
|
||||||
<span>Go to Google</span>
|
|
||||||
</AutoToolTipComponent>,
|
|
||||||
);
|
|
||||||
|
|
||||||
fireEvent.click(screen.getByText("Go to Google"));
|
|
||||||
expect(openSpy).toHaveBeenCalledWith("https://www.google.com", "_blank");
|
|
||||||
|
|
||||||
openSpy.mockRestore();
|
|
||||||
});
|
|
||||||
|
|
||||||
describe("isButtonTextTruncated", () => {
|
describe("isButtonTextTruncated", () => {
|
||||||
function mockElementWidths(
|
function mockElementWidths(
|
||||||
offsetWidth: number,
|
offsetWidth: number,
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,56 @@
|
||||||
|
import React from "react";
|
||||||
|
import { render, screen, fireEvent } from "@testing-library/react";
|
||||||
|
import "@testing-library/jest-dom";
|
||||||
|
import { BasicCell, type PropType } from "./BasicCell";
|
||||||
|
import { ColumnTypes } from "widgets/TableWidgetV2/constants";
|
||||||
|
import { CompactModeTypes } from "widgets/TableWidget/component/Constants";
|
||||||
|
|
||||||
|
describe("BasicCell Component", () => {
|
||||||
|
const defaultProps: PropType = {
|
||||||
|
value: "Test Value",
|
||||||
|
onEdit: jest.fn(),
|
||||||
|
isCellEditable: false,
|
||||||
|
hasUnsavedChanges: false,
|
||||||
|
columnType: ColumnTypes.TEXT,
|
||||||
|
url: "",
|
||||||
|
compactMode: CompactModeTypes.DEFAULT,
|
||||||
|
isHidden: false,
|
||||||
|
isCellVisible: true,
|
||||||
|
accentColor: "",
|
||||||
|
tableWidth: 100,
|
||||||
|
disabledEditIcon: false,
|
||||||
|
disabledEditIconMessage: "",
|
||||||
|
};
|
||||||
|
|
||||||
|
it("renders the value", () => {
|
||||||
|
render(<BasicCell {...defaultProps} />);
|
||||||
|
expect(screen.getByText("Test Value")).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("renders a link when columnType is URL", () => {
|
||||||
|
render(
|
||||||
|
<BasicCell
|
||||||
|
{...defaultProps}
|
||||||
|
columnType={ColumnTypes.URL}
|
||||||
|
url="http://example.com"
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
const link = screen.getByText("Test Value");
|
||||||
|
|
||||||
|
expect(link).toBeInTheDocument();
|
||||||
|
expect(link).toHaveAttribute("href", "http://example.com");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("calls onEdit when double-clicked", () => {
|
||||||
|
render(<BasicCell {...defaultProps} isCellEditable />);
|
||||||
|
fireEvent.doubleClick(screen.getByText("Test Value"));
|
||||||
|
expect(defaultProps.onEdit).toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("forwards ref to the div element", () => {
|
||||||
|
const ref = React.createRef<HTMLDivElement>();
|
||||||
|
|
||||||
|
render(<BasicCell {...defaultProps} ref={ref} />);
|
||||||
|
expect(ref.current).toBeInstanceOf(HTMLDivElement);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
@ -1,12 +1,13 @@
|
||||||
import type { Ref } from "react";
|
import type { Ref } from "react";
|
||||||
import React, { useCallback } from "react";
|
import React, { useCallback, useMemo } from "react";
|
||||||
import { Tooltip } from "@blueprintjs/core";
|
import { Tooltip } from "@blueprintjs/core";
|
||||||
import styled from "styled-components";
|
import styled from "styled-components";
|
||||||
import type { BaseCellComponentProps } from "../Constants";
|
import type { BaseCellComponentProps, CompactMode } from "../Constants";
|
||||||
import { TABLE_SIZES } from "../Constants";
|
import { TABLE_SIZES } from "../Constants";
|
||||||
import { TooltipContentWrapper } from "../TableStyledWrappers";
|
import { TooltipContentWrapper } from "../TableStyledWrappers";
|
||||||
import AutoToolTipComponent from "./AutoToolTipComponent";
|
import AutoToolTipComponent from "./AutoToolTipComponent";
|
||||||
import { importSvg } from "@appsmith/ads-old";
|
import { importSvg } from "@appsmith/ads-old";
|
||||||
|
import { ColumnTypes } from "widgets/TableWidgetV2/constants";
|
||||||
|
|
||||||
const EditIcon = importSvg(
|
const EditIcon = importSvg(
|
||||||
async () => import("assets/icons/control/edit-variant1.svg"),
|
async () => import("assets/icons/control/edit-variant1.svg"),
|
||||||
|
|
@ -55,7 +56,7 @@ const Content = styled.div`
|
||||||
const StyledEditIcon = styled.div<{
|
const StyledEditIcon = styled.div<{
|
||||||
accentColor?: string;
|
accentColor?: string;
|
||||||
backgroundColor?: string;
|
backgroundColor?: string;
|
||||||
compactMode: string;
|
compactMode: CompactMode;
|
||||||
disabledEditIcon: boolean;
|
disabledEditIcon: boolean;
|
||||||
}>`
|
}>`
|
||||||
position: absolute;
|
position: absolute;
|
||||||
|
|
@ -74,12 +75,12 @@ const StyledEditIcon = styled.div<{
|
||||||
}
|
}
|
||||||
`;
|
`;
|
||||||
|
|
||||||
type PropType = BaseCellComponentProps & {
|
export type PropType = BaseCellComponentProps & {
|
||||||
accentColor: string;
|
accentColor: string;
|
||||||
// TODO: Fix this the next time the file is edited
|
// TODO: Fix this the next time the file is edited
|
||||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||||
value: any;
|
value: any;
|
||||||
columnType: string;
|
columnType: ColumnTypes;
|
||||||
tableWidth: number;
|
tableWidth: number;
|
||||||
isCellEditable?: boolean;
|
isCellEditable?: boolean;
|
||||||
isCellEditMode?: boolean;
|
isCellEditMode?: boolean;
|
||||||
|
|
@ -128,6 +129,18 @@ export const BasicCell = React.forwardRef(
|
||||||
},
|
},
|
||||||
[onEdit, disabledEditIcon, isCellEditable],
|
[onEdit, disabledEditIcon, isCellEditable],
|
||||||
);
|
);
|
||||||
|
const contentToRender = useMemo(() => {
|
||||||
|
switch (columnType) {
|
||||||
|
case ColumnTypes.URL:
|
||||||
|
return (
|
||||||
|
<a href={url} rel="noopener noreferrer" target="_blank">
|
||||||
|
{value}
|
||||||
|
</a>
|
||||||
|
);
|
||||||
|
default:
|
||||||
|
return value;
|
||||||
|
}
|
||||||
|
}, [columnType, url, value]);
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<Wrapper
|
<Wrapper
|
||||||
|
|
@ -157,7 +170,7 @@ export const BasicCell = React.forwardRef(
|
||||||
url={url}
|
url={url}
|
||||||
verticalAlignment={verticalAlignment}
|
verticalAlignment={verticalAlignment}
|
||||||
>
|
>
|
||||||
<Content ref={contentRef}>{value}</Content>
|
<Content ref={contentRef}>{contentToRender}</Content>
|
||||||
</StyledAutoToolTipComponent>
|
</StyledAutoToolTipComponent>
|
||||||
{isCellEditable && (
|
{isCellEditable && (
|
||||||
<StyledEditIcon
|
<StyledEditIcon
|
||||||
|
|
|
||||||
|
|
@ -39,7 +39,7 @@ export type RenderDefaultPropsType = BaseCellComponentProps & {
|
||||||
// TODO: Fix this the next time the file is edited
|
// TODO: Fix this the next time the file is edited
|
||||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||||
value: any;
|
value: any;
|
||||||
columnType: string;
|
columnType: ColumnTypes;
|
||||||
tableWidth: number;
|
tableWidth: number;
|
||||||
isCellEditable: boolean;
|
isCellEditable: boolean;
|
||||||
isCellEditMode?: boolean;
|
isCellEditMode?: boolean;
|
||||||
|
|
|
||||||
|
|
@ -11,6 +11,7 @@ import {
|
||||||
} from "../Constants";
|
} from "../Constants";
|
||||||
import { CellWrapper } from "../TableStyledWrappers";
|
import { CellWrapper } from "../TableStyledWrappers";
|
||||||
import { BasicCell } from "./BasicCell";
|
import { BasicCell } from "./BasicCell";
|
||||||
|
import type { ColumnTypes } from "widgets/TableWidget/component/Constants";
|
||||||
|
|
||||||
const StyledSelectComponent = styled(SelectComponent)<{
|
const StyledSelectComponent = styled(SelectComponent)<{
|
||||||
accentColor: string;
|
accentColor: string;
|
||||||
|
|
@ -61,7 +62,7 @@ type SelectProps = BaseCellComponentProps & {
|
||||||
alias: string;
|
alias: string;
|
||||||
accentColor: string;
|
accentColor: string;
|
||||||
autoOpen: boolean;
|
autoOpen: boolean;
|
||||||
columnType: string;
|
columnType: ColumnTypes;
|
||||||
borderRadius: string;
|
borderRadius: string;
|
||||||
options?: DropdownOption[];
|
options?: DropdownOption[];
|
||||||
onFilterChange: (
|
onFilterChange: (
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user