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
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
29 changes: 1 addition & 28 deletions src/components/image/Image.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,11 @@ type Image = {
width: number;
height: number;
altText: string;
caption: string;
attribution: string;
alignment: "left" | "center" | "right";
};

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

const Images: FC<Image> = ({
image,
width,
height,
altText,
caption,
attribution,
alignment,
}) => {
const Images: FC<Image> = ({ image, width, height, altText, alignment }) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

Heard, those changes should be reflected in 58f993a

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 @@ -35,10 +22,6 @@ 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} />
</div>
<p className={`text-neutral-500 text-sm ${textAlignment}`}>
Photo Credit: {attribution}
</p>
<p className={`${textAlignment}`}>{caption}</p>
</div>
</div>
);
Expand All @@ -55,13 +38,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";
}
};