Events date range filter re-implementation with fix #707
Conversation
… missing in getEvents fn
…y params in the view all link that are supported by the events page + removing unused params (center and depth aren't supported or needed in getEvents) altogether
… with a modification to the useEffect which determines the matching quick filter -- this sets the current quick filter _and_ associated dates when there is no startDate or endDate passed
|
Preview deployment: https://events-date-range-filter.preview.avy-fx.org |
rchlfryn
left a comment
rchlfryn
left a comment
There was a problem hiding this comment.
One minor thing...I click Events in nav while on it, it should reset the quick date filter. It looks like the events update but we need to update the filter as well.
| )} | ||
| </div> | ||
| {eventOptions === 'static' && ( | ||
| {eventOptions === 'dynamic' && ( |
There was a problem hiding this comment.
Do we only want to show this for dynamic? It seems like it should be its own setting
There was a problem hiding this comment.
Hm yea that's interesting, we could have a "view all button" group with a label and link field. But the way it is right now, I think it only makes sense to show a view all button for dynamic because we can use the same params from the block in the /events?(params.toString()) url. For static options, where would the link go? We'd need the user to set that.
That could be a cool future enhancement. I'd want to ask current users if they would use that, though.
There was a problem hiding this comment.
This is the same logic as for the BlogList block: https://github.com/NWACus/web/blob/events-date-range-filter/src/blocks/BlogList/Component.tsx#L78 (although written differently because of the different way we query)
There was a problem hiding this comment.
Let's keep it as a future enhancement. I was just curious if this was something requested or just randomly decided
There was a problem hiding this comment.
Cool, yea just based on what made sense to me. We only have query params when using the 'dynamic' option and just making the button link to the /events page doesn't feel like it fits the use case of picking specific events.
Ah yes... that's because, this being a client side navigation, the |
…ext tick + moving no startDate && no endDate check outside of isInitialMount.current if block to handle all client side navigations to /events + matching against quick filters if only startDate is set and falling back to custom in that scenario
This was indeed more complicated than I initially realized. See 42196f0 for updates. What do you think about this approach? This works for page reloads, navigating to the events page via a Link component from other pages and also from the event page. |
|
This looks great!
This looks great! I think this approach will work. |
|
|
||
| const updateDateSelection = useCallback( | ||
| (filter: string, start: string, end: string) => { | ||
| console.log(`Setting filter to ${filter} and startDate: ${start} and endDate: ${end}.`) |
There was a problem hiding this comment.
| console.log(`Setting filter to ${filter} and startDate: ${start} and endDate: ${end}.`) |
There was a problem hiding this comment.
Ah a classic "forgot to remove my debug logs 🤦 " haha. Thanks.
There was a problem hiding this comment.
Oh hilarious - I'd already removed it and forgot to push. Even more classic 😏
Description
Re-implementing the
nuqs-based version of DateRangeFilter + fixes to the "View all" button on the EventList block.Key Changes
See commit messages.
Future enhancements / Questions
If you don't pass startDate or endDate to the /api/[center]/events endpoint you will now get future events only. In order to get past events you need to pass an endDate. I think this is only a problem if we wanted to get all past and future events since you would need to pass an endDate that includes all future events. But I think that's fine.