Skip to content

Conversation

chlowoee
Copy link
Contributor

@chlowoee chlowoee commented Jul 10, 2024

Why the changes are required?

To show which resources are made by DevSoc

Changes

  • Rearranged the resources cards so that CSESoc resources are on top
  • Added Made by CSESoc and Made by DevSoc (has hover animation and directs to DevSoc's website) above each society's resource cards.

Screenshots

Before
image
image

After
image
image

Comments

@chlowoee chlowoee changed the title [CW2-39] Adding DevSoc to Resources CW2-39 Adding DevSoc to Resources Jul 11, 2024
Copy link
Contributor

@derekxu04 derekxu04 left a comment

Choose a reason for hiding this comment

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

Nice work on your first ticket, looks really good!

Some nitpicks, otherwise happy to merge

@@ -3,8 +3,10 @@ import { resourceCards, stage1, stage2, stage3 } from '../../../public/data/reso

const boxStyling =
'border border-[#595F6D] rounded-lg hover:border-[#788093] hover:bg-[#070034] hover:bg-opacity-75 transition-all duration-300';
const socialsBoxStyling =
'xl:col-span-1 col-span-3 flex justify-center pt-2 pb-2 border border-[#595F6D] hover:border-[#788093] rounded-lg hover:bg-[#070034] hover:bg-opacity-75 transition-all duration-300';
// const socialsBoxStyling =
Copy link
Contributor

Choose a reason for hiding this comment

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

can remove

'xl:col-span-1 col-span-3 flex justify-center pt-2 pb-2 border border-[#595F6D] hover:border-[#788093] rounded-lg hover:bg-[#070034] hover:bg-opacity-75 transition-all duration-300';
// const socialsBoxStyling =
// 'xl:col-span-1 col-span-3 flex justify-center pt-2 pb-2 border border-[#595F6D] hover:border-[#788093] rounded-lg hover:bg-[#070034] hover:bg-opacity-75 transition-all duration-300';
const logostyle =
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this is only used once I think, you can probably just get rid of the variable and use it inline

@chlowoee chlowoee merged commit f4460d3 into master Jul 15, 2024
2 checks passed
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.

2 participants