Skip to content

Fix/event widget date filtering (2nd try)#253

Open
gxjansen wants to merge 3 commits into
mainfrom
fix/event-widget-date-filtering-new
Open

Fix/event widget date filtering (2nd try)#253
gxjansen wants to merge 3 commits into
mainfrom
fix/event-widget-date-filtering-new

Conversation

@gxjansen
Copy link
Copy Markdown
Contributor

@gxjansen gxjansen commented Apr 3, 2025

Issue

The event widget on the homepage was showing past events in the 'Upcoming' section because the date comparison was happening at build time rather than runtime.

Solution

Implemented client-side filtering for the event widget to ensure correct date comparison:

  1. Added a new function to fetch all events without date filtering
  2. Modified the VanillaEvents component to:
    • Fetch all events using the new function
    • Filter them into 'upcoming' and 'past' categories on the client side using the browser's current date
    • Sort them appropriately and limit each category to 3 events

This ensures that the date comparison happens in the browser at runtime, using the actual current date when a user visits the site, rather than the date at build time.

Testing

  • Verified that past events no longer appear in the 'Upcoming' section
  • Confirmed that the event widget still displays correctly with the new filtering logic

@gxjansen gxjansen requested a review from fsmeier April 3, 2025 12:51
@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 3, 2025

Deploy Preview for spryker-community ready!

Name Link
🔨 Latest commit 33e07b8
🔍 Latest deploy log https://app.netlify.com/projects/spryker-community/deploys/683dc70045a2180008108111
😎 Deploy Preview https://deploy-preview-253--spryker-community.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 69
Accessibility: 96
Best Practices: 92
SEO: 92
PWA: 90
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify project configuration.

]);

// Fetch all events
const events = await getAllEvents();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

when we can use the new Date() here, why dont we pass the date towards the function getAllEvents as parameter and really just query the needed timeframe?

Comment thread src/utils/vanillaApi.ts
}
}

export async function getAllEvents(): Promise<FormattedEvent[]> {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what about the other methods? do we still use them? If not i would propose to remove them because the more code we have the more we need to maintain over time and in a year from now we dont know much about the other code anymore.

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