Skip to content

Conversation

@hanminh1203
Copy link
Collaborator

@hanminh1203 hanminh1203 commented Dec 20, 2025

Change Summary

Improved responsive design and refactored artwork pages to follow Next.js best practices. Centralized mock data generation into a reusable hook and added proper error handling for empty database states.

Changes Made

  • Refactored ButtonGallery component to use Next.js Link component properly
  • Added placeholder SVG icon for artworks without images
  • Centralized mock data generation in useArtworkData.ts hook
  • Added error handling in getServerSideProps to return mock data when API fails or DB is empty
  • Improved responsive layouts with proper spacing and mobile-first approach
  • Fixed TypeScript type issues with Next.js route parameters and PageResult types

Change Form

  • The pull request title has an issue number (Individual art pages #8)
  • The change works by "Smoke testing" or quick testing
  • The change has tests - NA: Mock data hook doesn't require unit tests at this stage
  • The change has documentation - NA: Changes are self-explanatory code refactoring

Other Information

  • Add module django_filters.rest_framework - need confirmation

Testing Notes:

  • Tested with empty database - placeholder icons display correctly
  • Tested navigation between gallery and individual artwork pages
  • Verified responsive layout on mobile and desktop viewports

Related issue

Copy link
Contributor

@laurenpudz laurenpudz left a comment

Choose a reason for hiding this comment

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

Just a couple of notes regarding some (mostly) frontend things I've picked up on. I'd also like to get someone else's review on the backend work you've done. This PR is looking good though, keep up the good work :)

<div>
<div
data-layer="Right Panel"
className="RightPanel flex flex-col justify-start gap-2.5 py-5"
Copy link
Contributor

Choose a reason for hiding this comment

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

What effect does the RightPanel class have here?

data-layer="Right Panel"
className="RightPanel flex flex-col justify-start gap-2.5 py-5"
>
<div data-layer="Frame 1163" className="Frame1163 relative">
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be good to give the data-layer attribute a more descriptive name than "Frame 1163". We probably don't technically need the data-layer attributes but if you would like to keep them then please rename them to something more descriptive

<div data-layer="Frame 1163" className="Frame1163 relative">
<div
data-layer="Contributors"
className="Contributors text-light-3 justify-start font-['Jersey_10'] text-4xl font-normal tracking-wide"
Copy link
Contributor

Choose a reason for hiding this comment

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

You should be able to just use font-jersey10 here to change the font. Let me know if you have issues getting this to work and I can help sort it out

Comment on lines +70 to +94

.bg-neutral-1 {
background-color: var(--neutral-1);
}
.bg-dark-2 {
background-color: var(--dark-2);
}
.bg-light-2 {
background-color: var(--light-2);
}
.text-light-1 {
color: var(--light-1);
}
.outline-neutral-1 {
outline-color: var(--neutral-1);
}
.text-light-3 {
color: var(--light-3);
}
.border-light-2 {
border-color: var(--light-2);
}
.bg-error {
background-color: var(--error);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

You should be applying your styling using tailwind classes rather than defining your own custom css classes. e.g. you should be able to replicate the effects of .bg-neutral-1 class with the tailwind class background-neutral_1. You can find the variable names with which to refer to each colour in tailwind.config.ts. Please remove these and switch to using tailwind classes instead. Happy to help or explain if you need any assistance

- Changed Art model from CharField to ImageField for media
- Added ImageField with upload_to='art_images/'
- Created migration 0006 for field rename
- Added discord_url and instagram_url to ArtContributor model
- Created migration 0007 for social media fields
- Updated serializers for both Art and ArtContributor models
- Updated TypeScript types for Art model
- Added artwork index and detail pages with dynamic routing
- Replaced hardcoded SVG icons with react-social-icons library
- Configured Next.js to allow localhost images
- Fixed duplicate color definitions in globals.css
- Updated Tailwind config for consistent HSL colors
- Created test data script with sample images and social links
- Changed discord_url and instagram_url to use blank=True with default=''
- Removed null=True to follow Django best practices for string fields
- Updated migration 0007 accordingly
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.

Individual art pages

5 participants