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: Show nearby events on front page #1183

Draft
wants to merge 1 commit into
base: production
Choose a base branch
from
Draft

Conversation

iandunn
Copy link
Member

@iandunn iandunn commented Dec 13, 2023

Fixes #1160

This hides the current list of upcoming events by default, and adds a list of events that are geographically close to the user. There are buttons to toggle back and forth between the two views.

Those are fetched from api.w.org/events instead of directly querying wporg_events in order to take advantage of the geolocation logic that API has. The request is made via XHR in order to avoid a blocking HTTP request before the TTFB.

I don't expect a large number of cache hits on the Events API, since the payload includes the visitors (anonymized) IP. I think the API will be able to absorb the additional traffic without any problem, though, because of the relatively small amount of traffic on events.w.org.

[ todo add screenshots ]

@iandunn iandunn added this to the Events: Promotion milestone Dec 13, 2023
@iandunn iandunn self-assigned this Dec 13, 2023
@iandunn iandunn force-pushed the nearby-events branch 2 times, most recently from 15162cf to ef3cbbc Compare December 15, 2023 00:23
@iandunn iandunn force-pushed the nearby-events branch 3 times, most recently from 8fe0dbe to d3d58c7 Compare December 15, 2023 23:02
@iandunn
Copy link
Member Author

iandunn commented Dec 15, 2023

The functionality is working. Once the design is done I'll incorporate that.

Here's a few problems I've run into that I haven't found good solutions for:

  • Adding some kind of fade CSS transition when clicking the show/hide buttons. I played around with a few things, but the fact that display: block cancels transitions made it more complicated. I got it working with an overly complicated set of rules, but it didn't feel very elegant. There's gotta be a better way, but I didn't find it.
  • Avoiding a content jump when the API returns the results and they're injected into the DOM. I don't know the height because the number of events will be different for every visitor, so I just set it to the middle.
  • I had to make the chips an HTML block to avoid errors in the template editor. The <Button> block doesn't seem to support tagName: button, but a link isn't semantically appropriate. Maybe I should just remove the wp-block-buttons class since it's not providing much anyway, and write it all from scratch. It feels wrong to not reuse components that Core provides, though. I created a separate block for this. It's not elegant, but it works.
  • It's be more logical for the chips markup to be output by the event-list block, but AFAIK there isn't a way for a block to define a template part or some kind of reusable content like that. I could create an entirely new block, but that feels like overkill for such a small thing. I created a separate block for this. It's not elegant, but it works.
  • I didn't want to use JSX for view.js since that'd be an extra dependency, and wp.sanitize will return an unencoded string if the try fails, so I wrote a small wrapper to escape HTML. It seems correct to me, but it'd be nice to rely on Core/Gutenberg instead.

If anyone has any ideas for those I'd curious to hear them. @renintw @adamwoodnz @StevenDufresne

@StevenDufresne
Copy link
Contributor

Thanks for getting this PR up, it's helpful to see things in action.

I wonder if we should always show at least 10 events? I tested here in APAC and only had 1 result. That isn't very inspiring.

Maybe it isn't a toggle that we need but a sort?

Example
Screenshot 2023-12-18 at 11 41 17 AM

@iandunn
Copy link
Member Author

iandunn commented Dec 18, 2023

What we do in Core is to use that as an opportunity to encourage folks to organize. I really like that because it lets folks know that we're an open community, and that they don't have to wait for someone else to do it.

Screenshot 2023-12-18 at 9 44 40 AM Screenshot 2023-12-18 at 9 57 28 AM

If there aren't any events in their area, I don't think sorting the events outside their area (which could be hours away in many cases) really solves the problem of making the results relevant.

What do you think about that?

Also cc @fcoveram in case we want to change the design.

@fcoveram
Copy link

I shared an idea in the issue to keep design feedback and iteration over there.

@iandunn iandunn force-pushed the nearby-events branch 3 times, most recently from d891ccc to 4405d4a Compare December 22, 2023 00:34
@iandunn
Copy link
Member Author

iandunn commented Jan 30, 2024

The design team wants to wait on this for a bit, and I'll be on leave the next few months. If anyone wants to pick this up while I'm gone, here's an overview:

Functionality:

  • Most of it is in the event-list block.
  • The current render() is modularized so there are separate functions for nearby and global events
  • get_nearby_events_markup() is where the the payload for view.js is generated. Using the anonymized IP helps reuse any cache hits that are available on the web server and API server. 10 events should be fetched in case some of them are filtered out.
  • fetchLocalEvents is where the events are fetched from the API after the page loads. That avoids blocking the TTFB, and also allows passing the user's timezone to the API. That's necessary for reusing existing caches, and helps improve the accuracy of the geolocation.

Implementing the design:

  • Most of the UI work is in the event-list-chips block, but a lot of that may need to change based on where the design goes. I like the idea about a simpler homepage instead of the chips, but we'll need to wait and see what the design team wants to do.
  • The images/ folder is just placeholders, need to update when have final design
  • This may be a good candidate for using the interactivity API once that's stable. It'd probably simplify the UI boilerplate a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 📝 Draft
Development

Successfully merging this pull request may close these issues.

Events: Add geolocated events to Events landing page
4 participants