Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
3 changes: 3 additions & 0 deletions src/App.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Embed } from "./components/Embed/Embed";

Check failure on line 1 in src/App.tsx

View workflow job for this annotation

GitHub Actions / lint

'Embed' is defined but never used
import '@mantine/core/styles.css';
import '@mantine/dates/styles.css';

Expand All @@ -13,9 +14,11 @@

export default function App() {
return (

<Provider store={store}>
<MantineProvider theme={policyEngineTheme}>
<QueryClientProvider client={queryClient}>

<Router />
<ReactQueryDevtools initialIsOpen={false} />
</QueryClientProvider>
Expand Down
6 changes: 6 additions & 0 deletions src/Router.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
import { createBrowserRouter, RouterProvider } from 'react-router-dom';
import HomePage from './pages/Home.page';
import AIDemoPage from './pages/AIDemo.page';


const router = createBrowserRouter([
{
path: '/',
element: <HomePage />,
},
{
path: '/us/ai',
element: <AIDemoPage />,
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

issue, blocking: We're not looking to embed this as a page at this point.

Please remove this from the routes. We're looking to create reusable design components at the moment, not actually code pages yet.

]);

export function Router() {
Expand Down
88 changes: 88 additions & 0 deletions src/components/Embed/Embed.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import React from "react";
import { Card, Stack, Title, Text } from "@mantine/core";

Check failure on line 2 in src/components/Embed/Embed.tsx

View workflow job for this annotation

GitHub Actions / lint

'Text' is defined but never used

Check failure on line 2 in src/components/Embed/Embed.tsx

View workflow job for this annotation

GitHub Actions / lint

'Title' is defined but never used

Check failure on line 2 in src/components/Embed/Embed.tsx

View workflow job for this annotation

GitHub Actions / lint

'Stack' is defined but never used

Check failure on line 2 in src/components/Embed/Embed.tsx

View workflow job for this annotation

GitHub Actions / lint

'Card' is defined but never used

type Kind = "youtube" | "vimeo" | "iframe" | "image";
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit/suggestion: "kind" is a pretty non-standard term for something like this. "type" is more standard, but obviously quite vague and a reserved keyword. I'd recommend something like "embedType"


type Props = {
header: string;
kind: Kind;
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: Here, again, kind is fairly non-canonical. Would recommend something more standard.

src: string; // iframe src or image src
description?: string; // optional text under header
aspectRatio?: `${number} / ${number}` | number; // default 16/9
alt?: string; // required if kind="image"
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

issue: While you can describe this as a type, it's more standard to use an interface here

One drawback of types is that you lose the model name when using IntelliSense. Since you're not actually reusing this structure anywhere except the model of the inbound props, I'd recommend interface here


const fixSrc = (kind: Kind, src: string) => {
if (kind === "youtube") {
const id =
Copy link
Collaborator

Choose a reason for hiding this comment

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

suggestion/nit: ID is often used to refer to records in a database. Instead, call this link.

src.match(/(?:v=|youtu\.be\/|\/embed\/)([A-Za-z0-9_-]{6,})/)?.[1] ?? null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

suggestion: Define this regex elsewhere as a const and pass into .match() as an arg

It's tough to read these inline

return id ? `https://www.youtube.com/embed/${id}` : src;
}
if (kind === "vimeo") {
const id = src.match(/vimeo\.com\/(?:video\/)?(\d+)/)?.[1] ?? null;
return id ? `https://player.vimeo.com/video/${id}` : src;
}
return src;
};

export function Embed({
header,
kind,
src,
description,
aspectRatio = "16 / 9",
alt,
}: Props) {
const normalized = fixSrc(kind, src);
Copy link
Collaborator

Choose a reason for hiding this comment

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

suggestion/nit: Normalization often refers to structuring data in such a way that it's easy to find different record types by their IDs. Instead, I'd refer to this as "modified," I guess? Or something else.


return (
<div style={{ maxWidth: 960, margin: "0 auto" }}>
<h2 style={{ margin: "0 0 6px 0" }}>{header}</h2>
{description ? (
<p style={{ margin: "0 0 10px 0", color: "#6b7280" }}>{description}</p>
) : null}

<div
style={{
Copy link
Collaborator

Choose a reason for hiding this comment

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

issue, blocking: Please use Mantine v8 components when building. You can find their docs at https://mantine.dev/.

Please also use styling from the design framework. Our preference on styling is overrides to defaults -> defined "variants" in the styles folder -> local overrides

position: "relative",
width: "100%",
aspectRatio: typeof aspectRatio === "number" ? `${aspectRatio}` : aspectRatio,
overflow: "hidden",
borderRadius: 12,
background: "#00000010",
}}
>
{kind === "image" ? (
<img
src={src}
alt={alt ?? header}
style={{
position: "absolute",
inset: 0,
width: "100%",
height: "100%",
objectFit: "cover",
}}
loading="lazy"
/>
) : (
<iframe
title={header}
src={normalized}
allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture; web-share"
allowFullScreen
style={{
position: "absolute",
inset: 0,
width: "100%",
height: "100%",
border: 0,
}}
/>
)}
</div>
</div>
);
}

export default Embed;
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: Super minor, but my preference is to just use ES6+ "export default function Embed". Not necessary, though.

14 changes: 14 additions & 0 deletions src/pages/AIDemo.page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { Embed } from "../components/Embed/Embed";

export default function AIDemoPage() {
return (
<div style={{ padding: 24 }}>
<Embed
header="Watch Our AI Demo"
kind="youtube"
src="https://www.youtube.com/watch?v=fnuDyLKpt90" // replace with real ID
/>
</div>
);
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

issue, blocking: Please remove, as we are just looking to create generic components at the moment.

Loading