Skip to content

Conversation

burturt
Copy link
Member

@burturt burturt commented Jul 8, 2025

No description provided.

@burturt burturt requested a review from Copilot July 8, 2025 05:03
Copilot

This comment was marked as outdated.

@burturt burturt requested a review from Copilot July 8, 2025 05:14
Copilot

This comment was marked as outdated.

package.json Outdated
@@ -5,6 +5,7 @@
"scripts": {
"dev": "next dev",
"build": "next build",
"export": "next build && next export",
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to leave this in? It's unlikely that anyone will ever have to manually export the website, and next export should complain that you need to run next build first :D

);
}) => {
// Create a URL-friendly ID from the quarter name
const quarterId = quarter.name.toLowerCase().replace(/\s+/g, "-");
Copy link
Member

Choose a reason for hiding this comment

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

copilot, this is unnecessary; we currently throw away the original quarterId after parsing Cyanea information in data.tsx, but we should just preserve that information in the QuarterArchive interface and reuse it here

// Check tags/type
if (typeof event.type === "string") {
if (event.type.toLowerCase().includes(lowerQuery)) return true;
} else if (event.type !== null && event.type !== undefined) {
Copy link
Member

Choose a reason for hiding this comment

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

oops i changed this from Array.isArray earlier but now I realize that that is probably a better way of putting this condition

// Create a unique ID combining quarter and series names
const seriesId = `${quarterName}-${series.name}`
.toLowerCase()
.replace(/\s+/g, "-");
Copy link
Member

Choose a reason for hiding this comment

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

same here - use the original quarterId

@@ -20,9 +126,10 @@
border: var(--cyber-gold) 2px solid;
border-radius: 2rem;
width: 20rem;
height: 17rem;
min-height: 17rem; // Changed to min-height for flexibility
Copy link
Member

Choose a reason for hiding this comment

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

please remove copilot's "Increased" / "Changed" comments :D

max-width: 8rem;
}

// More specific selector to override .section span without !important
Copy link
Member

Choose a reason for hiding this comment

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

^

font-family: inherit; /* ??? override user-agent stylesheet for button */
text-align: left;
scroll-margin-top: 6rem; // Increased offset for fixed navbar
Copy link
Member

Choose a reason for hiding this comment

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

^


// Anchor link styling for quarters
h2 {
scroll-margin-top: 6rem; // Increased offset for fixed navbar
Copy link
Member

Choose a reason for hiding this comment

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

^


// Anchor link styling for series
h3 {
scroll-margin-top: 6rem; // Increased offset for fixed navbar
Copy link
Member

Choose a reason for hiding this comment

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

^

<div className={popUpStyle["popup"]}>
<div
className={popUpStyle["popup"]}
onClick={(e) => e.stopPropagation()} // Prevent closing when clicking inside popup
Copy link
Member

Choose a reason for hiding this comment

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

either remove this copilot comment or apply it to all of the onClick={(e) => e.stopPropagation()}

@burturt burturt marked this pull request as draft July 15, 2025 04:02
@Arc-blroth Arc-blroth marked this pull request as ready for review July 15, 2025 04:07
@burturt burturt requested a review from Copilot July 22, 2025 03:25
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements a comprehensive search and navigation system for the archive page, enhancing user experience with filtering capabilities and improved accessibility. The changes focus on making the archive more interactive and user-friendly while maintaining the existing cyberpunk aesthetic.

  • Adds a search functionality that filters events by title, description, and tags
  • Implements anchor links for quarters and series with smooth scrolling navigation
  • Enhances responsive design and accessibility features across components

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.

File Description
styles/Archive.module.scss Adds extensive styling for search components, anchor links, responsive design improvements, and better text overflow handling
pages/archive.tsx Implements search functionality, filtering logic, anchor link generation, and accessibility improvements including lazy loading
data/archive.ts Adds unique ID field to QuarterArchive interface to support anchor linking

@@ -118,7 +251,11 @@ const Event = ({
<span>
<img className={styles.icon} src="/images/slides.svg" />
Copy link
Preview

Copilot AI Jul 22, 2025

Choose a reason for hiding this comment

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

This image element is missing alt text and loading attributes that were added to other similar images in the same diff. This creates inconsistency in accessibility support.

Suggested change
<img className={styles.icon} src="/images/slides.svg" />
<img className={styles.icon} src="/images/slides.svg" alt="Slides icon" loading="lazy" />

Copilot uses AI. Check for mistakes.

@burturt burturt linked an issue Jul 22, 2025 that may be closed by this pull request
4 tasks
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.

Events page revamp
2 participants