Making WordPress.org

Opened 7 years ago

Closed 3 years ago

#2849 closed enhancement (reported-upstream)

Add sessions list and other details to speaker pages

Reported by: kau-boy's profile Kau-Boy Owned by: coreymckrill's profile coreymckrill
Milestone: Priority: low
Component: WordCamp Site & Plugins Keywords: needs-patch
Cc:

Description

For the new CampSite theme we had the idea to include a speakers bio page with an improved sessions list not only showing the session title, but also an excerpt as well as links to the slides and WordPress.tv video. We decided against a page template and rather add this additional markup to the post types plugin.

Attachments (3)

2849.diff (2.0 KB) - added by Kau-Boy 7 years ago.
2014-seatle-speakers-sessions-list-old-markup.PNG (28.4 KB) - added by Kau-Boy 6 years ago.
2014-seatle-speakers-sessions-list-new-markup.PNG (50.0 KB) - added by Kau-Boy 6 years ago.

Download all attachments as: .zip

Change History (19)

@Kau-Boy
7 years ago

#1 @coreymckrill
7 years ago

  • Keywords dev-feedback added
  • Owner set to coreymckrill
  • Status changed from new to accepted

#3 follow-up: @coreymckrill
7 years ago

@Kau-Boy @iandunn I'm a little concerned about back compatibility with this patch, since it will apply to all previous WC sites and it changes the markup that wraps the speaker session info, in addition to adding more session info.

Here is an example of the old markup:

<h2 class="speaker-sessions">Session</h2>
<ul id="speaker-session-names">
	<li><a href="http://2014.seattle.wordcamp.dev/session/seo-panel-discussion/">Making Sense of SEO for WordPress</a></li>
</ul>

And here is the same session with the new markup suggested in the patch:

<h2 class="speaker-sessions">Session</h2>
<div id="wcorg-session-807223" class="wcorg-session">
	<h3 class="wcorg-session-title"><a href="http://2014.seattle.wordcamp.dev/session/seo-panel-discussion/">Making Sense of SEO for WordPress</a></h3>
	<div class="wcorg-session-description">This panel discussion will focus on SEO issues specific to WordPress and, particularly, blogs. In addition, it will attempt to illuminate the lastest in Google goofiness and other peculiarities of SEO. It is not a basic tutorial in SEO, but is still a good choice for beginners.</div>
</div>

Couldn't we keep the <ul id="speaker-session-names"> wrapper and add the additional divs, etc. inside the <li> tags? That way we won't risk broken layout on any sites that have applied layout/styling to the session lists. Plus, I think keeping the top-level list makes more sense semantically since speakers can have more than one session.

#4 @Kau-Boy
7 years ago

I don't think that semantically it would make sense to keep the <code>ul</code>. I do agree, that there could be back compability issues, but keeping the same element and class, but with a different content, would rather enforce such issues. Skipping the class will more likely just render the list as it would naturally do and with the simple semantic (so list, just some h3 headlines) it would probably look better by default.

#5 in reply to: ↑ 3 @iandunn
7 years ago

Replying to coreymckrill:

Couldn't we keep the <ul id="speaker-session-names"> wrapper and add the additional divs, etc. inside the <li> tags?

That would be my first instinct, too.

Replying to Kau-Boy:

I don't think that semantically it would make sense to keep the <code>ul</code>.

Why not? We're displaying a list of sessions, so ul seems like the appropriate element to me. div is too generic to convey any semantic information, so I don't see how that'd be an improvement. Am I missing something?

I do agree, that there could be back compability issues, but keeping the same element and class, but with a different content, would rather enforce such issues. Skipping the class will more likely just render the list as it would naturally do and with the simple semantic (so list, just some h3 headlines) it would probably look better by default.

I don't think we should break back-compat that much. If we want to, we could set a feature flag to keep the old markup on existing sites, and use the new markup on new sites, but doesn't seem like an elegant solution.

It seems like Corey's idea about adding the new markup inside the li tags would satisfy the goal of this ticket without breaking back-compat. The speaker-session-names isn't exactly correct with the new markup, but it's close enough for me. Maintaining legacy code always involves some compromises.

#6 @coreymckrill
7 years ago

I would like to move forward with this ticket. @Kau-Boy are you ok with me adding the list wrapper back into your patch and committing it like that?

#7 @Kau-Boy
7 years ago

Sorry, I have been quite busy in the last days.

I still think, it's not a good idea, to use an ul as a wrapper. Worst case would be, that the Divs get bullet points or some other list symbols. Not using the list and its class would most likely just result in a nicely styled layout.

If you still think this would cause more back-compat issues, we really might want to use a feature flag for the moment and work on it together at the WCEU Contributor Day.

#8 @melchoyce
7 years ago

  • Keywords ui-feedback added

Can we get some screenshots of the latest patch?

#9 @iandunn
6 years ago

  • Summary changed from Improve the sessions list on the speakers bio page to Add sessions list and other details to speaker pages

#10 @iandunn
6 years ago

  • Priority changed from normal to low

This ticket was mentioned in Slack in #meta-wordcamp by kau-boy. View the logs.


6 years ago

#12 @melchoyce
6 years ago

Thanks for the screenshots, @Kau-Boy. If we're making tweaks to this, any chance we can increase the scope a tiny bit and wrap "Sessions" in a header? (Whatever hierarchically comes after whatever the speaker name is wrapped in.)

Might also be explicitly worth including a "read more" or "read the full session description" at the end of the excerpt.

This ticket was mentioned in Slack in #meta-wordcamp by iandunn. View the logs.


5 years ago

#14 @iandunn
5 years ago

  • Keywords needs-patch added; has-patch dev-feedback ui-feedback removed

Slack summary:

We discussed some options with using blocks, but it needs more discussion and thought.

If we do make markup changes, then incorporating the header per Mel sounds like a good idea.

Either way the "read more" link sounds good.

This ticket was mentioned in Slack in #meta-wordcamp by ryelle. View the logs.


4 years ago

#16 @dd32
3 years ago

  • Resolution set to reported-upstream
  • Status changed from accepted to closed

This ticket has been moved to GitHub https://github.com/WordPress/wordcamp.org/issues/653

Note: See TracTickets for help on using tickets.