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: Use a showcase like approach for the events list/archive #1124

Merged
merged 30 commits into from
Dec 6, 2023

Conversation

StevenDufresne
Copy link
Contributor

@StevenDufresne StevenDufresne commented Dec 4, 2023

This is a draft PR exploring the use of components from https://github.com/wordpress/wporg-showcase-2022 to build out the "All Events" page. This is a Work in Progress.

Why I think this is the best approach

  • We don't have to get the google map abstraction perfect since we don't need to do it.
    • The google map block is already more than a google map block.
  • By using a very similar approach to showcase, we'll get almost all styles out of the box
    • We're talking implementation in hours, not days (it took me less than an hour to put this together)
  • Consistency

What's not perfect here yet

  • This currently filters on the full list of retrieved events.
    • May be a bad idea if there are too many events
  • The event retrieval functions are in the google map block plugin.
  • By rendering from the server, we don't get the best user-centric datetime for the user.

Current

Note: the filters are not being applied to the results set yet.

events wordpress test__map_type%5B%5D=wordcamp format_type%5B%5D=wordcamp s=asdfasf

Copy link
Member

@iandunn iandunn left a comment

Choose a reason for hiding this comment

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

This WFM 👍🏻

Note: the filters are not being applied to the results set yet.

This was the part that didn't feel great to me, since it's shoehorning arbitrary data from the wporg_events table into a WP_Query flow. The opposite approach in WordPress/wporg-mu-plugins#523 has it's own set of problems, though, so I think this PR is at least as good, and probably better.

iandunn pushed a commit that referenced this pull request Dec 5, 2023
Try using a php block that queries.

Example query filtering.

add block build script.

Add the patterns

Hook in another filter.

Rename template.

Some minor updates.

Add the month filter
@iandunn iandunn mentioned this pull request Dec 5, 2023
@iandunn
Copy link
Member

iandunn commented Dec 5, 2023

I merged in the Past Events template in 9cc0b23

@StevenDufresne
Copy link
Contributor Author

Opened up a PR for adjusting the time block:
WordPress/wporg-mu-plugins#534

@@ -46,7 +46,7 @@
<!-- wp:column {"width":"33.33%","fontSize":"medium","fontFamily":"eb-garamond"} -->
<div class="wp-block-column has-eb-garamond-font-family has-medium-font-size" style="flex-basis:33.33%"><!-- wp:group {"style":{"border":{"radius":"2px","width":"1px"},"spacing":{"padding":{"left":"var:preset|spacing|30","top":"var:preset|spacing|20","bottom":"var:preset|spacing|20"}}},"borderColor":"light-grey-1","layout":{"type":"constrained"}} -->
<div class="wp-block-group has-border-color has-light-grey-1-border-color" style="border-width:1px;border-radius:2px;padding-top:var(--wp--preset--spacing--20);padding-bottom:var(--wp--preset--spacing--20);padding-left:var(--wp--preset--spacing--30)"><!-- wp:paragraph {"style":{"layout":{"selfStretch":"fit","flexSize":null},"elements":{"link":{"color":{"text":"var:preset|color|charcoal-0"}}}},"textColor":"charcoal-0","fontSize":"heading-6"} -->
<p class="has-charcoal-0-color has-text-color has-link-color has-heading-6-font-size">540,537 members</p>
<p class="has-charcoal-0-color has-text-color has-link-color has-heading-6-font-size">540,537 attendees</p>
Copy link
Member

Choose a reason for hiding this comment

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

I don't think it'd be accurate to change this. That number is the # of meetup.com group members (plus WordCamp attendees, but that doesn't affect it significantly), and the number of attendees is likely to be significantly different.

If we really want to say attendees, we could try to estimate the # based on RSVPs, but I don't think it'd be accurate enough to publish. IMO members is the most accurate/meaningful stat we can use here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for that context. I was going off the designs. I'll provide that feedback and switch it back to members today.

Copy link

Choose a reason for hiding this comment

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

IMO members is the most accurate/meaningful stat we can use here.

I definitely see what you mean. I'm just unsure if "members", despite being accurate, is the right term to use. It does have hints of exclusivity and seems directly tied to Meetup.com's terminology, not necessarily WordPress's. When "members" are talked about in WP ways, I've typically seen it explicitly contextualized as "community members"... which is too long (as usual 😅).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So summing up the problem, if we use "attendees", we will be underrepresenting the actual number of attendees.

I also find members to be an inclusive term. What about just people?

Simple is as simple does.

Copy link

Choose a reason for hiding this comment

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

people is accurate and simple, for sure. Also feels too generic/passive. (and what of all the pet contributors? 😎)

I'd suggest one of the following (for now), with my preference being 1:

  1. participants: technically accurate, the bar for participation can be fluid, i.e., as low as just becoming a member on Meetup.com or as high as attending and contributing at a WordCamp.
  2. contributors: perhaps not entirely inclusive/accurate, but directly related to the project and also used elsewhere on the page already.

Copy link
Member

Choose a reason for hiding this comment

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

🤔 that's a good point, we definitely don't want it to feel exclusionary. I like participants, thanks for thinking of that!

I think the problem with attendees is more that it _over_represents the number of attendees, since so many people join a meetup group, and even RSVP to an event, but don't actually show up. IMO accuracy is the most important factor, and it'd be better to err on the side of too low than too high. My concern is that inflating stats, even unintentionally, would erode trust.

I'll go ahead and update it to participants since we're coming up on the deadline to launch, but I'm happy to continue discussing it if y'all would like to iterate on that, or make any changes.

$events = get_events( $attributes['events'], 0, 0, $facets );

// Get all the filters that are currently applied.
$filtered_events = array_slice( filter_events( $events ), 0, 10);
Copy link
Member

Choose a reason for hiding this comment

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

This is cutting the list off at 10, but we only want that for the homepage. Maybe the block should accept a limit attribute that defaults to 50 or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, good idea. I was thinking about the grouping as well. Should we pass in a groupyByMonth property? Feels too specific.

Alternatively, we can pass a "heading" property to this list block and create a wrapper block where we use the query results to loop and output multiple instances of the event list block. I'll look at that today.

<div class="wp-block-group alignwide"
style="padding-right:var(--wp--preset--spacing--edge-space);padding-bottom:var(--wp--preset--spacing--50);padding-left:var(--wp--preset--spacing--edge-space)">

<!-- wp:pattern {"slug":"wporg-events-2023/event-list-filters-archive"} /-->
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't exist in the repo, do you have it locally? Or should this be event-list-filters?

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 as #1124 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm, I was on late last night. Fuzzy brain. Forgot to add it to git.

Copy link
Member

Choose a reason for hiding this comment

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

Does it still need to be added? I just worked around it in 303e77f, but I'm not sure if that matched what you have locally

<div class="wp-block-group"><!-- wp:search {"showLabel":false,"placeholder":"Search events...","width":100,"widthUnit":"%","buttonText":"Search","buttonPosition":"button-inside","buttonUseIcon":true,"className":"is-style-secondary-search-control"} /--></div>
<!-- /wp:group -->

<!-- wp:template-part {"slug":"event-filters"} /-->
Copy link
Member

Choose a reason for hiding this comment

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

I don't see this either, do you have it locally?

Copy link
Member

Choose a reason for hiding this comment

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

I added it in 303e77f so I can work on it today, but free free to change that if you had something else in mind

@iandunn iandunn marked this pull request as ready for review December 6, 2023 00:46
@iandunn iandunn merged commit 673c20c into production Dec 6, 2023
3 checks passed
@iandunn iandunn deleted the try/new-block branch December 6, 2023 00:46
@iandunn iandunn added this to the Events: Promotion milestone Dec 11, 2023
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.

3 participants