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

Features/workshop page #37

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Features/workshop page #37

wants to merge 12 commits into from

Conversation

andrewtarnavsky
Copy link

Workshop Page

Added Workshops from last year as a template to update later when this years workshops are out and ready.

andrewtarnavsky and others added 9 commits March 18, 2024 23:55
Created an array for workshop info and added pics for each workshop.
Mapped out all workshops and added a card and modal styling as well as a hover effect
Added the nav bar and footer to the page and changed up some fonts and colors.
Added transition effects to the detailed view when entering and exiting. Changed the font color of the workshop property strings, as well as the detailed view background color, and all other text inside the detailed view to white.
Updated the hover effect to also change the background opacity of the card so that it darkens when hovering over it.
Updated to use NextImage and fixed problem where the aspect ratios of each image werent the same
Comment on lines 3 to 14
import workshopMern from "./../public/workshopPics/mern.png";
import workshopPassword from "./../public/workshopPics/password.jpeg";
import workshopHacking from "./../public/workshopPics/hacking.jpeg";
import workshopSafety from "./../public/workshopPics/safety.jpeg";
import workshopDesign from "./../public/workshopPics/design.jpeg";
import workshopAI from "./../public/workshopPics/ai.jpeg";
import workshopAgile from "./../public/workshopPics/agile.jpeg";
import workshopForge from "./../public/workshopPics/forge.png";
import workshopMobdev from "./../public/workshopPics/mobdev.png";
import workshopQuantum from "./../public/workshopPics/quantum.png";
import workshopSemi from "./../public/workshopPics/semi.jpeg";
import workshopTechstack from "./../public/workshopPics/techstack.jpeg";
Copy link
Member

Choose a reason for hiding this comment

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

Can you remove this and use NextImage's picture loading from URL like you did for the meet the team page?

@@ -0,0 +1,312 @@
import React, { useState, useEffect, useRef } from "react";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
import React, { useState, useEffect, useRef } from "react";
import React, { useState } from "react";

description: string;
}

const workshops = [
Copy link
Member

Choose a reason for hiding this comment

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

Can you move this to a JSON file in the public folder? I think that is a better place for information like this.

Comment on lines +206 to +231
<style>{`
.workshop-container {
transition: transform 0.3s ease; /* Original transition */
position: relative;
}

.workshop-container:hover .bg-overlay {
background-color: rgba(34, 34, 34, 0.7); /* Darker background color */
}

.bg-overlay {
position: absolute;
inset: 0;
background-color: rgba(34, 34, 34, 0); /* Initial background color */
transition: background-color 0.4s ease; /* Background color transition */
pointer-events: none;
z-index: -1; /* Ensure the overlay is behind the content */
}

.image-container {
width: 100%;
padding-top: 60.25%; /* 16:9 aspect ratio (more rectangular) */
position: relative;
overflow: hidden;
}
`}</style>
Copy link
Member

@CooperW824 CooperW824 Apr 2, 2024

Choose a reason for hiding this comment

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

Can you convert this to TailwindCSS? For some of those more arbitrary values check out how to use arbitrary values in Tailwind. Arbitrary Values

Comment on lines +234 to +238
<br />
<br />
<br />
<br />
<br />
Copy link
Member

Choose a reason for hiding this comment

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

this is really bad practice, can you find another way to do this maybe using margin or padding?

<br />
<br />
<h1 className="font-helvetica text-white text-6xl mb-10 text-center">Checkout our Workshops!</h1>
<br />
Copy link
Member

Choose a reason for hiding this comment

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

Also see if you can replace this, you shouldn't use <br> for padding.


.image-container {
width: 100%;
padding-top: 60.25%; /* 16:9 aspect ratio (more rectangular) */
Copy link
Member

Choose a reason for hiding this comment

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

there is a better way to do this using CSS Aspect Ratio

.foo{
aspect-ratio: 16 / 9;
}

Tailwind supports this too with aspect-video

<br />
<div className="grid grid-cols-1 md:grid-cols-3 gap-16">
{workshops.map((workshop, index) => (
<div key={index} className="mb-8 relative">
Copy link
Member

Choose a reason for hiding this comment

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

This div feels unnecessary

<div className="cursor-pointer" onClick={() => handleShow(workshop)}>
<div className="group relative">
<div className="workshop-container rounded-lg overflow-hidden shadow-lg transform transition-transform duration-300 hover:scale-105">
<div className="bg-overlay"></div>
Copy link
Member

Choose a reason for hiding this comment

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

This div might also be uncessary and you can move this css to other divs.

>
<h2 className="text-2xl font-bold mb-4">{selectedWorkshop.title}</h2>
<p>
<strong className="text-green-200 font-bold">Time:</strong>{" "}
Copy link
Member

@CooperW824 CooperW824 Apr 2, 2024

Choose a reason for hiding this comment

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

can you use one of the HackRPI colors for these text segments maybe the light-blue or light-green?

<div className="bg-transparent p-4 text-center">
<h2 className="text-white">{workshop.title}</h2>
<p className="text-sm">
<span className="text-green-200 font-bold">Time:</span>{" "}
Copy link
Member

Choose a reason for hiding this comment

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

same with these text segments, can you change the text color?

<br />
<h1 className="font-helvetica text-white text-6xl mb-10 text-center">Checkout our Workshops!</h1>
<br />
<div className="grid grid-cols-1 md:grid-cols-3 gap-16">
Copy link
Member

Choose a reason for hiding this comment

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

This is a bit jarring to go from 1 col to 3 cols maybe add a sm:grid-cols-2?

Copy link
Member

@CooperW824 CooperW824 left a comment

Choose a reason for hiding this comment

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

Overall, this looks pretty good so far, there are some small changes to make just to improve code clarity and design. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants