WordPress.org

Making WordPress.org

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#774 closed enhancement (fixed)

Custom field for link to slides

Reported by: BrashRebel Owned by: iandunn
Milestone: Priority: normal
Component: WordPress.tv Keywords: has-patch dev-feedback
Cc:

Description

The WPTV mod squad recently discussed the idea of adding a field specifically for a URL to presentation slides to the WPTV workflow. The reason for this is twofold:

  1. It would simplify and streamline the workflow for moderators by providing a more intuitive and faster mechanism for including a reference to a presentation's slides. Currently links to slides are manually created within the_excerpt which means they cannot be called directly (if that were desired at some point) and are also a nuisance for moderators to generate.
  2. It would make it easier and more obvious to video submitters to include the URLs to slides in the first place.

To that end, I just finished a proposed solution which I've attached and does the following:

  • Adds an additional field to the Submit a Video form
  • Posts that value to the anon-upload.php form so that moderators can approve it
  • Adds the value to a new custom field called _wptv_slides_url upon approval
  • Appends a link to the_excerpt using the prescribed format if a value exists

Thanks for feedback and I'm certainly open to alternative solutions if my proposed course isn't ideal.

Attachments (2)

diff.diff (178.3 KB) - added by BrashRebel 5 years ago.
774.diff (8.8 KB) - added by BrashRebel 5 years ago.

Download all attachments as: .zip

Change History (11)

@BrashRebel
5 years ago

#1 @iandunn
5 years ago

That sounds like a good approach to me. It looks like diff.diff contains the entire theme, rather than just the changes, though. Can you recreate it with just the changes?

It's always good to manually review the svn diff output before adding a patch, because it's easy to accidentally leave in some debugging code, or for your IDE to change the formatting of unrelated lines, etc.

#2 @BrashRebel
5 years ago

  • Cc kyle@… added

Sorry about that. I'm still new to this. I created a new diff which includes every commit after the initial one so it should have everything I did.

@BrashRebel
5 years ago

#3 @iandunn
5 years ago

  • Owner set to iandunn
  • Resolution set to fixed
  • Status changed from new to closed

In 1044:

WordPress.tv: Add meta field for presentation slides.

Fixes #774
props BrashRebel

#4 follow-up: @iandunn
5 years ago

No worries, there's lots of little things that everyone encounters when they first start contributing. The patch looks really good, though :)

Ideally we probably want to display the slides more prominently than just appending them to the excerpt -- like maybe their own h5 section in the sidebar -- but since all the existing posts use the excerpt method, I think appending the new value to the excerpt is a smart choice. The new and old ones will match for now, and in the future we can migrate the old date to the new field and update the design to include it.

There were a few places where the new value wasn't being escaped, so I fixed those. We can't assume that anything pulled from the database is safe -- even if it was validated before being put there -- since it could have been compromised in the mean time.

#5 in reply to: ↑ 4 @JerrySarcastic
5 years ago

  • Cc jerry@… added

Replying to iandunn:

Ideally we probably want to display the slides more prominently than just appending them to the excerpt -- like maybe their own h5 section in the sidebar

I think this is a great idea! It's worth noting though that we don't get slides from every speaker or for every WordCamp talk (such as those with a live demo) so we would want to be able to hide that heading for videos that don't have slides.

#6 @iandunn
5 years ago

Yup, that'd be easy :)

#7 @JerrySarcastic
5 years ago

Is it possible to modify this slide link to add the following to the link?

target='_blank'

It would be good to have the slides open in a new tab, as it is likely the visitor is in the act of viewing a video when they click this link, and may not want to be taken off the page.

This ticket was mentioned in Slack in #wptv by jerrysarcastic. View the logs.


5 years ago

#9 @iandunn
5 years ago

It's an easy change to make, but we don't normally do it because it's considered a bad practice. This use case might be an exception to that rule, though. There was some discussion at https://wordpress.slack.com/archives/wptv/p1420129455000011, but we wanted to let a few others weigh in. Do you have any thoughts on the specific points we discussed there?

Note: See TracTickets for help on using tickets.