Skip to content

Conversation

@chadbrokaw
Copy link
Collaborator

This pull request introduces significant improvements to the event calendar UI, focusing on enhanced user experience and modernizing analytics integration. The main changes include replacing Google Analytics UA tracking with Google Tag Manager, redesigning the calendar's visual style, and adding new modular React components for better event browsing and filtering.

Calendar UI Enhancements:

  • Added new modular React components:
    • EventSlideout: Provides a detailed slide-out panel for single or multiple events, with accessibility and focus management.
    • MobileAgenda: Offers a mobile-friendly agenda view with intuitive navigation and event chips.
    • CategoryFilters: Enables filtering events by category with interactive chips.
    • CustomToolbar: Implements a custom toolbar with navigation controls and a centered label for the calendar.
  • Updated styles in EventCalendar.module.scss for a cleaner, more modern look:
    • Changed event header from a gradient card to a simple border-bottom.
    • Reduced category badge size for better visual balance.
    • Added styling for new filter and instruction elements. [1] [2] [3]

Analytics Integration:

  • Replaced the legacy react-ga (Google Analytics UA) with react-gtm-module (Google Tag Manager) for analytics tracking, in anticipation of GA UA deprecation. [1] [2] [3]

Supporting Utilities and Constants:

  • Introduced a color palette and day mapping constants in constants.ts to support category coloring and day calculations in the calendar.

These changes collectively modernize the calendar's codebase, improve accessibility and usability, and ensure compliance with updated analytics practices.

@chadbrokaw chadbrokaw changed the title Development fix: Implement GA and fix up calendar Sep 29, 2025
Copy link

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 pull request modernizes the event calendar UI and updates analytics tracking from Google Analytics UA to Google Tag Manager. The changes include significant refactoring to improve modularity, user experience, and compliance with updated analytics practices.

  • Replaces legacy Google Analytics UA with Google Tag Manager for enhanced analytics capabilities
  • Refactors the event calendar into modular React components with improved accessibility and mobile support
  • Enhances error handling and provides better fallback states for PageNotFound scenarios

Reviewed Changes

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

Show a summary per file
File Description
package.json Adds react-gtm-module and @types/react-gtm-module dependencies
app/index.html Removes inline Google Tag Manager script
app/App.tsx Switches from react-ga4 to react-gtm-module initialization
app/pages/HomePage/HomePage.tsx Updates calendar import path and adds ErrorBoundary wrapper
app/components/ui/PageNotFound.tsx Adds typed error states for different not-found scenarios
app/pages/*/ Updates PageNotFound imports to use new typed system
app/components/ui/ErrorBoundary/ Adds new error boundary component with retry functionality
app/components/ui/Calendar/ Complete calendar refactor into modular components with improved state management

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines 28 to 30
const r = Math.max(0, parseInt(hex.substr(0, 2), 16) - 40);
const g = Math.max(0, parseInt(hex.substr(2, 2), 16) - 40);
const b = Math.max(0, parseInt(hex.substr(4, 2), 16) - 40);
Copy link

Copilot AI Sep 29, 2025

Choose a reason for hiding this comment

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

The substr() method is deprecated. Use substring() or slice() instead for better compatibility.

Suggested change
const r = Math.max(0, parseInt(hex.substr(0, 2), 16) - 40);
const g = Math.max(0, parseInt(hex.substr(2, 2), 16) - 40);
const b = Math.max(0, parseInt(hex.substr(4, 2), 16) - 40);
const r = Math.max(0, parseInt(hex.substring(0, 2), 16) - 40);
const g = Math.max(0, parseInt(hex.substring(2, 4), 16) - 40);
const b = Math.max(0, parseInt(hex.substring(4, 6), 16) - 40);

Copilot uses AI. Check for mistakes.
Comment on lines +1055 to +1056
margin-top: 8px;
margin-bottom: 24px;
Copy link

Copilot AI Sep 29, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider using consistent spacing units throughout the stylesheet. The 8px and 24px values should ideally use a spacing scale (e.g., 0.5rem and 1.5rem) for better maintainability.

Suggested change
margin-top: 8px;
margin-bottom: 24px;
margin-top: 0.5rem;
margin-bottom: 1.5rem;

Copilot uses AI. Check for mistakes.
Comment on lines 30 to 35
setTimeout(() => {
const closeBtn = document.querySelector(
".close-slideout-btn"
) as HTMLElement;
closeBtn?.focus();
}, 100);
Copy link

Copilot AI Sep 29, 2025

Choose a reason for hiding this comment

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

Using setTimeout for focus management is unreliable and can cause accessibility issues. Consider using a ref to focus the element directly or useLayoutEffect for synchronous DOM updates.

Copilot uses AI. Check for mistakes.
Comment on lines 31 to 33
const closeBtn = document.querySelector(
".close-slideout-btn"
) as HTMLElement;
Copy link

Copilot AI Sep 29, 2025

Choose a reason for hiding this comment

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

Direct DOM querying with querySelector is not recommended in React. Use a ref to access DOM elements instead for better maintainability and type safety.

Copilot uses AI. Check for mistakes.
@chadbrokaw chadbrokaw merged commit 0be0424 into main Sep 29, 2025
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