Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fleet UI: Clean up TabNav and TargetChipSelector components #26256

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@
// @ts-ignore
import InputField from "components/forms/fields/InputField";
import TooltipWrapper from "components/TooltipWrapper";
import TabsWrapper from "components/TabsWrapper";
import TabsWrapper from "components/TabNav";
import InfoBanner from "components/InfoBanner/InfoBanner";
import CustomLink from "components/CustomLink/CustomLink";
import Radio from "components/forms/fields/Radio";
import TabText from "components/TabText";

import { isValidPemCertificate } from "../../../pages/hosts/ManageHostsPage/helpers";
import IosIpadosPanel from "./IosIpadosPanel";
Expand Down Expand Up @@ -56,9 +57,9 @@
interface IPlatformWrapperProps {
enrollSecret: string;
onCancel: () => void;
certificate: any;

Check warning on line 60 in frontend/components/AddHostsModal/PlatformWrapper/PlatformWrapper.tsx

View workflow job for this annotation

GitHub Actions / lint-js (ubuntu-latest)

Unexpected any. Specify a different type
isFetchingCertificate: boolean;
fetchCertificateError: any;

Check warning on line 62 in frontend/components/AddHostsModal/PlatformWrapper/PlatformWrapper.tsx

View workflow job for this annotation

GitHub Actions / lint-js (ubuntu-latest)

Unexpected any. Specify a different type
config: IConfig | null;
}

Expand Down Expand Up @@ -584,7 +585,7 @@
// so we add a hidden pseudo element with the same text string
return (
<Tab key={navItem.name} data-text={navItem.name}>
{navItem.name}
<TabText>{navItem.name}</TabText>
</Tab>
);
})}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
.platform-wrapper {
padding: 0; // different to pad sticky subnav properly

.component__tabs-wrapper {
.tab-nav {
// negate problematic sticky positioning
position: initial;
.react-tabs {
Expand Down
47 changes: 1 addition & 46 deletions frontend/components/LiveQuery/SelectTargets.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,29 +25,17 @@ import teamsAPI, { ILoadTeamsResponse } from "services/entities/teams";
import { formatSelectedTargetsForApi } from "utilities/helpers";
import { capitalize } from "lodash";
import permissions from "utilities/permissions";
import {
LABEL_DISPLAY_MAP,
PlatformLabelNameFromAPI,
} from "utilities/constants";

import PageError from "components/DataError";
import TargetsInput from "components/LiveQuery/TargetsInput";
import Button from "components/buttons/Button";
import Spinner from "components/Spinner";
import TooltipWrapper from "components/TooltipWrapper";
import Icon from "components/Icon";
import SearchField from "components/forms/fields/SearchField";
import RevealButton from "components/buttons/RevealButton";
import TargetPillSelector from "./TargetChipSelector";
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to separate directory/file to add unit tests, storybook, organize styling

import { generateTableHeaders } from "./TargetsInput/TargetsInputHostsTableConfig";

interface ITargetPillSelectorProps {
entity: ISelectLabel | ISelectTeam;
isSelected: boolean;
onClick: (
value: ISelectLabel | ISelectTeam
) => React.MouseEventHandler<HTMLButtonElement>;
}

interface ISelectTargetsProps {
baseClass: string;
queryId?: number | null;
Expand Down Expand Up @@ -86,11 +74,6 @@ const STALE_TIME = 60000;
const SECTION_CHARACTER_LIMIT = 600;

const isLabel = (entity: ISelectTargetsEntity) => "label_type" in entity;
const isBuiltInLabel = (
entity: ISelectTargetsEntity
): entity is ISelectLabel & { label_type: "builtin" } => {
return "label_type" in entity && entity.label_type === "builtin";
};
const isAllHosts = (entity: ISelectTargetsEntity) =>
"label_type" in entity &&
entity.name === "All Hosts" &&
Expand Down Expand Up @@ -125,34 +108,6 @@ const getTruncatedEntityCount = (
return index;
};

const TargetPillSelector = ({
entity,
isSelected,
onClick,
}: ITargetPillSelectorProps): JSX.Element => {
const displayText = (): string => {
if (isBuiltInLabel(entity)) {
const labelName = entity.name as PlatformLabelNameFromAPI;
if (labelName in LABEL_DISPLAY_MAP) {
return LABEL_DISPLAY_MAP[labelName] || labelName;
}
}

return entity.name || "Missing display name";
};

return (
<button
className="target-pill-selector"
data-selected={isSelected}
onClick={(e) => onClick(entity)(e)}
>
<Icon name={isSelected ? "check" : "plus"} />
<span className="selector-name">{displayText()}</span>
</button>
);
};

const SelectTargets = ({
baseClass,
queryId,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
import React from "react";
import { Meta, StoryObj } from "@storybook/react";
import { ISelectLabel, ISelectTeam } from "interfaces/target";
import TargetChipSelector from "./TargetChipSelector"; // Adjust the path if necessary

const meta: Meta<typeof TargetChipSelector> = {
component: TargetChipSelector,
title: "Components/TargetChipSelector",
argTypes: {
entity: {
description: "The label or team entity to display.",
control: { type: "object" },
},
isSelected: {
description:
"Whether the chip is currently selected, updated by parent onClick handler.",
control: { type: "boolean" },
},
onClick: {
description: "The handler to call when the chip is clicked.",
action: "clicked", // Use Storybook's action to track clicks
},
},
parameters: {
backgrounds: {
default: "light",
values: [
{ name: "light", value: "#ffffff" },
{ name: "dark", value: "#333333" },
],
},
},
};

export default meta;

type Story = StoryObj<typeof TargetChipSelector>;

// Example data for labels and teams
const mockLabel: ISelectLabel = {
id: 1,
name: "Example Label",
label_type: "regular",
description: "A test label",
};

const mockTeam: ISelectTeam = {
id: 2,
name: "Example Team",
description: "A test team",
};

export const LabelExample: Story = {
args: {
entity: mockLabel,
isSelected: false,
onClick: (value) => (event) => {
event.preventDefault();
console.log("Clicked label:", value);
},
},
render: (args) => (
<TargetChipSelector
entity={args.entity}
isSelected={args.isSelected}
onClick={args.onClick}
/>
),
};

export const TeamExample: Story = {
args: {
entity: mockTeam,
isSelected: true,
onClick: (value) => (event) => {
event.preventDefault();
console.log("Clicked team:", value);
},
},
render: (args) => (
<TargetChipSelector
entity={args.entity}
isSelected={args.isSelected}
onClick={args.onClick}
/>
),
};

export const BuiltInLabelExample: Story = {
args: {
entity: {
id: 3,
name: "MS Windows",
label_type: "builtin",
description: "Microsoft Windows hosts",
},
isSelected: false,
onClick: (value) => (event) => {
event.preventDefault();
console.log("Clicked label:", value);
},
},
render: (args) => (
<TargetChipSelector
entity={args.entity}
isSelected={args.isSelected}
onClick={args.onClick}
/>
),
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
import React from "react";
import { render, screen, fireEvent } from "@testing-library/react";

import { ISelectLabel, ISelectTeam } from "interfaces/target";
import TargetChipSelector from "./TargetChipSelector";

describe("TargetChipSelector", () => {
const mockOnClick = jest.fn();

const mockLabel: ISelectLabel = {
id: 1,
name: "Example Label",
label_type: "regular",
description: "A test label",
};

const mockTeam: ISelectTeam = {
id: 2,
name: "Example Team",
description: "A test team",
};

it("renders the correct display text for a label", () => {
render(
<TargetChipSelector
entity={mockLabel}
isSelected={false}
onClick={mockOnClick}
/>
);

expect(screen.getByText("Example Label")).toBeInTheDocument();
});

it("renders the correct display text for a team", () => {
render(
<TargetChipSelector
entity={mockTeam}
isSelected={false}
onClick={mockOnClick}
/>
);

expect(screen.getByText("Example Team")).toBeInTheDocument();
});

it("renders the correct icon when selected", () => {
render(
<TargetChipSelector entity={mockLabel} isSelected onClick={mockOnClick} />
);

expect(screen.getByLabelText("check")).toBeInTheDocument();
});

it("renders the correct icon when not selected", () => {
render(
<TargetChipSelector
entity={mockLabel}
isSelected={false}
onClick={mockOnClick}
/>
);

expect(screen.getByLabelText("plus")).toBeInTheDocument();
});

it("calls the onClick handler with the correct entity when clicked", () => {
render(
<TargetChipSelector
entity={mockLabel}
isSelected={false}
onClick={(value) => (event) => mockOnClick(value, event)}
/>
);

fireEvent.click(screen.getByRole("button"));

expect(mockOnClick).toHaveBeenCalledWith(mockLabel, expect.any(Object));
});

it("applies the correct data-selected attribute when selected", () => {
render(
<TargetChipSelector entity={mockLabel} isSelected onClick={mockOnClick} />
);

const button = screen.getByRole("button");
expect(button).toHaveAttribute("data-selected", "true");
});

it("applies the correct data-selected attribute when not selected", () => {
render(
<TargetChipSelector
entity={mockLabel}
isSelected={false}
onClick={mockOnClick}
/>
);

const button = screen.getByRole("button");
expect(button).toHaveAttribute("data-selected", "false");
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import React from "react";
import {
ISelectLabel,
ISelectTeam,
ISelectTargetsEntity,
} from "interfaces/target";
import Icon from "components/Icon";
import {
PlatformLabelNameFromAPI,
LABEL_DISPLAY_MAP,
} from "utilities/constants";

interface ITargetChipSelectorProps {
entity: ISelectLabel | ISelectTeam;
isSelected: boolean;
onClick: (
value: ISelectLabel | ISelectTeam
) => React.MouseEventHandler<HTMLButtonElement>;
}

const isBuiltInLabel = (
entity: ISelectTargetsEntity
): entity is ISelectLabel & { label_type: "builtin" } => {
return "label_type" in entity && entity.label_type === "builtin";
};

const TargetChipSelector = ({
entity,
isSelected,
onClick,
}: ITargetChipSelectorProps): JSX.Element => {
const displayText = (): string => {
if (isBuiltInLabel(entity)) {
const labelName = entity.name as PlatformLabelNameFromAPI;
if (labelName in LABEL_DISPLAY_MAP) {
return LABEL_DISPLAY_MAP[labelName] || labelName;
}
}

return entity.name || "Missing display name";
};

return (
<button
className="target-chip-selector"
data-selected={isSelected}
onClick={(e) => onClick(entity)(e)}
>
<Icon name={isSelected ? "check" : "plus"} />
<span className="selector-name">{displayText()}</span>
</button>
);
};

export default TargetChipSelector;
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
.target-pill-selector {
.target-chip-selector {
padding: $pad-small;
background-color: $core-white;
border: none;
Expand Down Expand Up @@ -34,7 +34,7 @@
}

&:hover {
box-shadow: inset 0 0 0 1px $core-vibrant-blue-over;
background-color: $ui-vibrant-blue-10;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed hover state per figma design


&:active {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default } from "./TargetChipSelector";
Loading
Loading