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.

Sign up for GitHub

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

Jump to bottom

Feature: About page (title, description, and image) #12

Merged
merged 6 commits into from
Oct 20, 2023
Merged

Feature: About page (title, description, and image) #12

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0c5365e
About page draft
Phantom0110 Oct 15, 2023
5ffb313
Fix requested changes
Phantom0110 Oct 17, 2023
ad37c8f
Fix lint error and add newline
Phantom0110 Oct 17, 2023
989e992
Change page classNames to match CSS classes
Brian-Magnuson Oct 18, 2023
2f25de8
Change CSS classNames to use camelCase
Brian-Magnuson Oct 18, 2023
6b86834
Rename group_photo for about page and update alt text
Brian-Magnuson Oct 20, 2023
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
5 changes: 5 additions & 0 deletions next.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
/** @type {import('next').NextConfig} */
const nextConfig = {
output: "export",
images: {
// Next JS Image Optimization does not work on static site generation
// See: https://nextjs.org/docs/pages/building-your-application/optimizing/images
unoptimized: true
}
};

module.exports = nextConfig;
Binary file added public/group_photo.jpg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
30 changes: 30 additions & 0 deletions src/app/about/About.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
.about {
/* styles */
Brian-Magnuson marked this conversation as resolved.
Show resolved Hide resolved
text-align: center;
}

.about__image_container {
Copy link
Member

Choose a reason for hiding this comment

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

[Nit]: As this is in About.module.css, there's no need to use an about__ prefix. Also, prefer kebab-case for HTML/CSS identifiers (id/class) as with URLs (not fully sure the origin for this though).

Copy link
Contributor

Choose a reason for hiding this comment

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

CSS classNames have been changed to use camelCase as recommended by documentation: https://github.com/css-modules/css-modules#naming

display: flex;
justify-content: center;
margin-bottom: 2rem;
Brian-Magnuson marked this conversation as resolved.
Show resolved Hide resolved
margin-top: 2rem;

}

.about__title {
margin-bottom: 5rem;
font-size: 3rem;
font-family: fantasy;
Brian-Magnuson marked this conversation as resolved.
Show resolved Hide resolved
}

.about__description_container {
width: 100%;
Brian-Magnuson marked this conversation as resolved.
Show resolved Hide resolved

}

.about__description {
margin: 0 auto;
max-width: 1000px;
Copy link
Member

Choose a reason for hiding this comment

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

[Comment]: I see the idea here. Normally, you would have some type of container class that would handle margin/padding for the page rather than apply it to specific elements (there's a few other tricks too for this to work better with mobile too). Anyways, keep this for now and we'll figure out a more general solution for all pages later as part of global styles.

font-size: 1rem;
Brian-Magnuson marked this conversation as resolved.
Show resolved Hide resolved
font-family: Cambria, Cochin, Georgia, Times, 'Times New Roman', serif;
Brian-Magnuson marked this conversation as resolved.
Show resolved Hide resolved
}
Brian-Magnuson marked this conversation as resolved.
Show resolved Hide resolved
21 changes: 21 additions & 0 deletions src/app/about/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import Image from "next/image";
import styles from "./About.module.css";

export default function About() {
return (
<main className={styles.about}>
<h1 className={styles.about__title}>About</h1>
<div className={styles.about__description_container}>
<p className={styles.about__description}>We are an organization that helps students learn and apply software engineering principles to real-world applications. We host weekly workshops on topics like software design to help bridge the gap between what students need to know for industry and what they're taught in classes. These concepts help with building complex software systems and better prepare members for team projects, internships, and careers in software development.

</p>
</div>

<div className={styles.about__image_container}><Image src="group_photo.jpg" alt="A group of club members sitting in a room looking at the camera and posing" height={600} width={600} />
Copy link
Member

Choose a reason for hiding this comment

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

[Suggestion]: We can probably get a bit better with the name/alt - I happen to know this photo is from the Fall 2022 planning meeting. Suggestion:

fall_2022_planning_meeting.jpg
SSD members (about 30) at our Fall 2022 planning meeting in CISE.

I think this better represents the meaning of the photo rather than just being a description, but open to suggestions. Mainly looking ahead to when we (hopefully) have a gallery of images and need to do a better job sorting through that.

Copy link
Contributor

Choose a reason for hiding this comment

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

@WillBAnders The alt text has since been updated. However, the name of the file is "group_photo.jpg", not "fall_2022_planning_meeting.jpg". Will this discrepancy be an issue?

Copy link
Member

Choose a reason for hiding this comment

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

No, I would just recommend the more explicit name to help if/when we add a gallery with multiple pictures where it becomes more important to distinguish each one. But the file can always be renamed at the time; it's more of a callout to prefer more detailed names when possible.

</div>

</main>


);
}
Loading