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

Events slide component implementation #229

Merged
merged 3 commits into from
Feb 25, 2025
Merged

Events slide component implementation #229

merged 3 commits into from
Feb 25, 2025

Conversation

sun-dazer
Copy link
Collaborator

Issues #199 and #142

  • Added links for all available slides (Fall 2023)
  • Added captions and cover images (cover images on UploadThing)
  • Fixed aspect ratio and modal sizing
  • Added toggle between years and toggle between slides and recording (need to figure out how recordings are stored)

Copy link

vercel bot commented Feb 13, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
uf-sase-website 🛑 Canceled (Inspect) Feb 14, 2025 0:01am

@T-Joseph-Kim
Copy link
Collaborator

Looks good to me! Thanks mia.

Copy link
Collaborator

@T-Joseph-Kim T-Joseph-Kim left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Collaborator

Choose a reason for hiding this comment

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

The slides load kind of slow, so you can use prefetch or preload to have them ready quicker
There's also an x in the middle of the slides. Maybe some accidental or incorrect model nesting

Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably use slidesData instead of imageData. Could also have an option to search through captions (should be called eventName?), but that can be implemented later.
I thought it would give a warning but seo is not used so it should be deleted/commented

@TheRickyZhang TheRickyZhang merged commit 1a5b579 into main Feb 25, 2025
7 checks passed
TheRickyZhang added a commit that referenced this pull request Feb 25, 2025
TheRickyZhang added a commit that referenced this pull request Feb 25, 2025
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.

3 participants