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

Move Breadcrumbs and reconfigure for V2Routes #1550

Merged
merged 5 commits into from
Mar 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion ui/components/AutomationsTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { Automation } from "../hooks/automations";
import { formatURL } from "../lib/nav";
import { AutomationType, V2Routes } from "../lib/types";
import DataTable, { SortType } from "./DataTable";
import KubeStatusIndicator from "./KubeStatusIndicator";
import KubeStatusIndicator, { computeReady } from "./KubeStatusIndicator";
import Link from "./Link";

type Props = {
Expand Down Expand Up @@ -57,6 +57,8 @@ function AutomationsTable({ className, automations }: Props) {
a.conditions.length > 0 ? (
<KubeStatusIndicator conditions={a.conditions} />
) : null,
sortType: SortType.bool,
sortValue: ({ conditions }) => computeReady(conditions),
},
{
label: "Revision",
Expand Down
44 changes: 44 additions & 0 deletions ui/components/Breadcrumbs.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import React from "react";
import styled from "styled-components";
import useNavigation from "../hooks/navigation";
import { getPageLabel, getParentNavValue, V2Routes } from "../lib/nav";
import Flex from "./Flex";
import Icon, { IconType } from "./Icon";
import Link from "./Link";
import Text from "./Text";

const CrumbLink = styled(Link)`
${Text} {
font-size: ${(props) => props.theme.fontSizes.large};
color: ${(props) => props.theme.colors.neutral00};
}
`;

export const Breadcrumbs = () => {
const { currentPage } = useNavigation();
const parentValue = getParentNavValue(currentPage);

return (
<Flex align>
{parentValue !== currentPage && (
<CrumbLink
to={V2Routes[parentValue as V2Routes] || ""}
textProps={{ bold: true }}
>
{getPageLabel(parentValue as V2Routes)}
</CrumbLink>
)}
{parentValue !== currentPage && (
<Icon type={IconType.NavigateNextIcon} size="large" color="neutral00" />
)}
<CrumbLink
to={currentPage}
textProps={parentValue === currentPage && { bold: true }}
>
{getPageLabel(currentPage)}
</CrumbLink>
</Flex>
);
};

export default styled(Breadcrumbs)``;
5 changes: 2 additions & 3 deletions ui/components/DataTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export interface Props {
/** A list of objects with four fields: `label`, which is a string representing the column header, `value`, which can be a string, or a function that extracts the data needed to fill the table cell, `sortType`, which determines the sorting function to be used, and `sortValue`, which customizes your input to the search function */
fields: Field[];
/** A list of data that will be iterated through to create the columns described in `fields`. */
rows: any[];
rows?: any[];
/** index of field to initially sort against. */
defaultSort?: number;
/** an optional list of string widths for each field/column. */
Expand Down Expand Up @@ -83,8 +83,7 @@ function defaultSortFunc(sort: Field): Sorter {

export const sortWithType = (rows: Row[], sort: Field) => {
const sortFn = sort.sortValue || defaultSortFunc(sort);

return rows.sort((a: Row, b: Row) => {
return (rows || []).sort((a: Row, b: Row) => {
switch (sort.sortType) {
case SortType.number:
return sortFn(a) - sortFn(b);
Expand Down
7 changes: 6 additions & 1 deletion ui/components/Layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ import { FeatureFlags } from "../contexts/FeatureFlags";
import useNavigation from "../hooks/navigation";
import { formatURL, getParentNavValue } from "../lib/nav";
import { V2Routes } from "../lib/types";
import Breadcrumbs from "./Breadcrumbs";
import Flex from "./Flex";
import Link from "./Link";
import Logo from "./Logo";
import Spacer from "./Spacer";
import UserSettings from "./UserSettings";

type Props = {
Expand Down Expand Up @@ -131,12 +133,15 @@ const TopToolBar = styled(Flex)`
function Layout({ className, children }: Props) {
const { authFlag } = React.useContext(FeatureFlags);
const { currentPage } = useNavigation();

return (
<div className={className}>
<AppContainer>
<TopToolBar between align>
<TopToolBar start align>
<Logo />
{authFlag ? <UserSettings /> : null}
<Spacer padding="xxl" />
<Breadcrumbs />
</TopToolBar>
<Main wide>
<NavContainer>
Expand Down
10 changes: 8 additions & 2 deletions ui/components/Link.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as React from "react";
import { Link as RouterLink } from "react-router-dom";
import styled from "styled-components";
import Text from "./Text";
import Text, { TextProps } from "./Text";

type Props = {
className?: string;
Expand All @@ -10,6 +10,7 @@ type Props = {
children?: any;
href?: any;
newTab?: boolean;
textProps?: TextProps;
onClick?: () => void;
};

Expand All @@ -20,9 +21,14 @@ function Link({
to = "",
newTab,
onClick,
textProps,
...props
}: Props) {
const txt = <Text color="primary">{children}</Text>;
const txt = (
<Text color="primary" {...textProps}>
{children}
</Text>
);

if (href) {
return (
Expand Down
24 changes: 1 addition & 23 deletions ui/components/Page.tsx
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
import { Breadcrumbs } from "@material-ui/core";
import _ from "lodash";
import React from "react";
import styled from "styled-components";
import useCommon from "../hooks/common";
import { formatURL } from "../lib/nav";
import { PageRoute, RequestError } from "../lib/types";
import Alert from "./Alert";
import Flex from "./Flex";
import Footer from "./Footer";
import Link from "./Link";
import LoadingPage from "./LoadingPage";
import Spacer from "./Spacer";

Expand Down Expand Up @@ -52,16 +49,6 @@ export const TitleBar = styled.div`
}
`;

function pageLookup(p: PageRoute) {
switch (p) {
case PageRoute.Applications:
return "Applications";

default:
break;
}
}

function Errors({ error }) {
const arr = _.isArray(error) ? error : [error];
return (
Expand All @@ -79,7 +66,6 @@ function Page({
className,
children,
title,
breadcrumbs,
actions,
loading,
error,
Expand All @@ -98,15 +84,7 @@ function Page({
<div className={className}>
<Content>
<TitleBar>
<Breadcrumbs>
{breadcrumbs &&
_.map(breadcrumbs, (b) => (
<Link key={b.page} to={formatURL(b.page, b.query)}>
<h2>{pageLookup(b.page)}</h2>
</Link>
))}
<h2>{title}</h2>
</Breadcrumbs>
<h2>{title}</h2>
{actions}
</TitleBar>
{error && <Errors error={error} />}
Expand Down
6 changes: 5 additions & 1 deletion ui/components/ReconciledObjectsTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,15 @@ function ReconciledObjectsTable({
},
{
label: "Type",
value: (u: UnstructuredObject) => `${u.groupVersionKind.kind}`,
value: (u: UnstructuredObject) => u.groupVersionKind.kind,
sortType: SortType.string,
sortValue: (u: UnstructuredObject) => u.groupVersionKind.kind,
},
{
label: "Namespace",
value: "namespace",
sortType: SortType.string,
sortValue: ({ namespace }) => namespace,
Copy link
Contributor

Choose a reason for hiding this comment

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

No longer necessary, right?

746b1be

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My branch was out of date :( will fix

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ALSO - noticed a problem with this - right now DataTable is using the field having a SortType as an indication of whether it's sortable or not - we need a different indication of which columns are sortable

Copy link
Contributor

Choose a reason for hiding this comment

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

@joshri Still not fixed?

},
{
label: "Status",
Expand Down
6 changes: 3 additions & 3 deletions ui/components/Text.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,17 @@ import styled from "styled-components";
// eslint-disable-next-line
import { colors, fontSizes } from "../typedefs/styled";

type Props = {
export interface TextProps {
className?: string;
size?: keyof typeof fontSizes;
bold?: boolean;
semiBold?: boolean;
capitalize?: boolean;
italic?: boolean;
color?: keyof typeof colors;
};
}

const Text = styled.span<Props>`
const Text = styled.span<TextProps>`
font-family: ${(props) => props.theme.fontFamilies.regular};
font-size: ${(props) => props.theme.fontSizes[props.size]};
font-weight: ${(props) => {
Expand Down
31 changes: 31 additions & 0 deletions ui/components/__tests__/Breadcrumbs.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import React from "react";
import renderer from "react-test-renderer";
import { createMockClient, withContext, withTheme } from "../../lib/test-utils";
import Breadcrumbs from "../Breadcrumbs";

describe("snapshots", () => {
it("renders", () => {
const tree = renderer
joshri marked this conversation as resolved.
Show resolved Hide resolved
.create(
withTheme(
withContext(<Breadcrumbs />, "/automations", {
jpellizzari marked this conversation as resolved.
Show resolved Hide resolved
applicationsClient: createMockClient({}),
})
)
)
.toJSON();
expect(tree).toMatchSnapshot();
});
it("renders child route", () => {
const tree = renderer
.create(
withTheme(
withContext(<Breadcrumbs />, "/kustomization?name=flux", {
applicationsClient: createMockClient({}),
})
)
)
.toJSON();
expect(tree).toMatchSnapshot();
});
});
68 changes: 68 additions & 0 deletions ui/components/__tests__/__snapshots__/Breadcrumbs.test.tsx.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`snapshots renders 1`] = `
<div
className="sc-bdvvtL lacWsj"
>
<a
className="sc-hKwDye sc-eCImPb kCA-dBN cQVkzF"
href="/automations"
onClick={[Function]}
>
<span
className="sc-gsDKAQ klFDZy"
color="primary"
size="normal"
>
Applications
</span>
</a>
</div>
`;

exports[`snapshots renders child route 1`] = `
<div
className="sc-bdvvtL lacWsj"
>
<a
className="sc-hKwDye sc-eCImPb kCA-dBN cQVkzF"
href="/"
onClick={[Function]}
>
<span
className="sc-gsDKAQ klFDZy"
color="primary"
size="normal"
>
Applications
</span>
</a>
<div
className="sc-bdvvtL lacWsj sc-dkPtRN hTjxvJ"
>
<svg
aria-hidden={true}
className="MuiSvgIcon-root"
focusable="false"
viewBox="0 0 24 24"
>
<path
d="M10 6L8.59 7.41 13.17 12l-4.58 4.59L10 18l6-6z"
/>
</svg>
</div>
<a
className="sc-hKwDye sc-eCImPb kCA-dBN cQVkzF"
href="/kustomization"
onClick={[Function]}
>
<span
className="sc-gsDKAQ fcdFvu"
color="primary"
size="normal"
>
flux
</span>
</a>
</div>
`;
14 changes: 1 addition & 13 deletions ui/components/__tests__/__snapshots__/Page.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -106,19 +106,7 @@ exports[`Page snapshots default 1`] = `
<div
className="c2"
>
<nav
className="MuiTypography-root MuiBreadcrumbs-root MuiTypography-body1 MuiTypography-colorTextSecondary"
>
<ol
className="MuiBreadcrumbs-ol"
>
<li
className="MuiBreadcrumbs-li"
>
<h2 />
</li>
</ol>
</nav>
<h2 />
</div>
<div
className="c3"
Expand Down
19 changes: 19 additions & 0 deletions ui/lib/nav.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import qs from "query-string";
import { useLocation } from "react-router-dom";
import { SourceRefSourceKind } from "./api/core/types.pb";
import { PageRoute } from "./types";

Expand Down Expand Up @@ -41,6 +42,24 @@ export const getParentNavValue = (
}
};

export const getPageLabel = (currentPage: string): string => {
const parsed = qs.parse(useLocation().search);
Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 Not sure how this is working. I was under the impression that you cannot call hooks from plain old JS like this. I thought it had to be within another hook.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Me too but it works so I'm not asking questions

switch (currentPage) {
case V2Routes.Automations:
return "Applications";
case V2Routes.Sources:
return "Sources";
case V2Routes.FluxRuntime:
return "Flux Runtime";
case V2Routes.Kustomization:
case V2Routes.HelmRelease:
case V2Routes.GitRepo:
case V2Routes.HelmChart:
case V2Routes.Bucket:
return parsed.name as string;
}
};

export const formatURL = (page: string, query: any = {}) => {
return `${page}?${qs.stringify(query)}`;
};
Expand Down