Skip to content
Open
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
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
"eslint": "8.57.1",
"eslint-config-love": "101.0.0",
"eslint-config-next": "15.0.3",
"eslint-config-love": "101.0.0",
"eslint-plugin-import": "2.31.0",
"eslint-plugin-n": "17.14.0",
"eslint-plugin-promise": "7.2.1",
Expand Down
2 changes: 1 addition & 1 deletion src/components/about-us/BoardMembers.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import ImageComponent from "@/components/image/Image";
import ImageComponent from "@/components/image/ImageWithCaption";
import { FC } from "react";
import boardSrc from "../../../public/images/about-us/board.jpg";

Expand Down
36 changes: 13 additions & 23 deletions src/components/image/Image.tsx
Original file line number Diff line number Diff line change
@@ -1,29 +1,26 @@
import { FC } from "react";
import { CSSProperties, FC } from "react";
import Image from "next/image";

type Image = {
image: string;
width: number;
height: number;
altText: string;
caption: string;
attribution: string;
sizes?: string;
style?: CSSProperties;
alignment: "left" | "center" | "right";
};

// const [isHovering, setIsHovering] = useState(false)

const Images: FC<Image> = ({
image,
width,
height,
altText,
caption,
attribution,
alignment,
sizes,
style,
}) => {
let flexAlignment = getFlexAlignment(alignment);
let textAlignment = getTextAlignment(alignment);

return (
<div className={`flex ${flexAlignment}`}>
Copy link
Contributor

Choose a reason for hiding this comment

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

question: Is it possible to use RadixUI's Flex component here instead of Tailwind? Maybe have a flex parameter which is used to pass thru relevant attributes using spreading? Might only want to allow a subset of them (i.e. let the calling code handle margin / padding if it's needed, or only allow attributes relevant for a flex with one element).

Expand All @@ -33,12 +30,15 @@ const Images: FC<Image> = ({
}}
>
<div className="relative">
Copy link
Contributor

Choose a reason for hiding this comment

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

question: Does this div do anything? Not sure why that was there originally.

<Image src={image} alt={altText} height={height} width={width} />
<Image
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: Would recommend keeping pass-thru attribute names aligned (i.e. src and alt). I realize the names were already different- can we include updating those in this PR?

src={image}
alt={altText}
height={height}
width={width}
sizes={sizes}
style={style}
/>
</div>
<p className={`text-neutral-500 text-sm ${textAlignment}`}>
Photo Credit: {attribution}
</p>
<p className={`${textAlignment}`}>{caption}</p>
</div>
</div>
);
Expand All @@ -55,13 +55,3 @@ export const getFlexAlignment = (alignment: string) => {
return "justify-center";
}
};

export const getTextAlignment = (alignment: string) => {
if (alignment === "right") {
return "text-right";
} else if (alignment === "left") {
return "text-left";
} else {
return "text-center";
}
};
67 changes: 67 additions & 0 deletions src/components/image/ImageWithCaption.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import { FC } from "react";
import Image from "next/image";

type ImageWithCaption = {
image: string;
width: number;
height: number;
altText: string;
caption?: string;
attribution?: string;
alignment: "left" | "center" | "right";
};

const ImagesWithCaption: FC<ImageWithCaption> = ({
image,
width,
height,
altText,
caption,
attribution,
alignment,
}) => {
let flexAlignment = getFlexAlignment(alignment);
let textAlignment = getTextAlignment(alignment);

return (
<div className={`flex ${flexAlignment}`}>
<div
style={{
width: width,
}}
>
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Use RadixUI's Box component here? That'd enable us to use their mobile-responsive breakpoints, etc.

<div className="relative">
<Image src={image} alt={altText} height={height} width={width} />
Copy link
Contributor

Choose a reason for hiding this comment

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

question: Would we want to pass thru sizes & styles like the above component does?

</div>
{attribution && (
<p className={`text-neutral-500 text-sm ${textAlignment}`}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: Use RadixUI's Text component here, and for the caption below.

Photo Credit: {attribution}
</p>
)}
{caption && <p className={`${textAlignment}`}>{caption}</p>}
</div>
</div>
);
};

export default ImagesWithCaption;

export const getFlexAlignment = (alignment: string) => {
if (alignment === "right") {
return "justify-end";
} else if (alignment === "left") {
return "justify-start";
} else {
return "justify-center";
}
};

export const getTextAlignment = (alignment: string) => {
if (alignment === "right") {
return "text-right";
} else if (alignment === "left") {
return "text-left";
} else {
return "text-center";
}
};