Skip to content

CW2-39 Adding DevSoc to Resources #22

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

Merged
merged 2 commits into from
Jul 15, 2024
Merged

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