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

UX design for embeded video on posts #1452

Closed
willdoran opened this issue Oct 17, 2016 · 11 comments
Closed

UX design for embeded video on posts #1452

willdoran opened this issue Oct 17, 2016 · 11 comments
Assignees

Comments

@willdoran
Copy link
Contributor

willdoran commented Oct 17, 2016

Expected behaviour

  • The user should be able to provide a link to a youtube or vimeo video, select its type and save
  • The detail page should display the video embed in an iframe

Actual behaviour

@rjmackay
Copy link
Contributor

As mentioned on our call ideally this shouldn't extend the media input type, rather it should extend varchar but have a custom validator and input.

@willdoran
Copy link
Contributor Author

@rjmackay yep, though this ticket is I think for mocking a UI for the video input

@brandonrosage
Copy link

@willdoran I intend to stage a flow that removes the need to select a service (e.g. YouTube), and instead sniffs the URL you provide and does the rest. If it can't sniff a known service, it'll ask the user to include all the embed HTML.

I'll do the sniffing in the PL for YouTube and Vimeo. But is that kosher?

@willdoran
Copy link
Contributor Author

At the moment there is a function which converts the URLs given to the
embed format, each one has a slightly different format. Pulling the
source directly out of the URL is a good idea. We can clean them or
provided html before saving it. It's a bit dodgy to allow users to
insert html code. I'll check if we can adequately sanitize it to make it
save. It's a good idea though so I think it's worth trying to get it to
work.

On 2016-10-19 19:40, Brandon Rosage wrote:

@willdoran https://github.com/willdoran I intend to stage a flow
that removes the need to select a service (e.g. YouTube), and instead
sniffs the URL you provide and does the rest. If it can't sniff a
known service, it'll ask the user to include all the embed HTML.

I'll do the sniffing in the PL for YouTube and Vimeo. But is that kosher?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1452 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACkdBWB5PLX9Wn3_p9saWYXEvsgTV0RUks5q1qqEgaJpZM4KZHI2.

@rjmackay
Copy link
Contributor

I'd be very nervous about allowing users to give us the embed HTML. I'd rather we just supported a set list of services and let users know if they URL they provide doesn't match.

@brandonrosage
Copy link

@jshorland, @willdoran, and @rjmackay: How do y'all feel about the following UX?

Video embed demo

@willdoran
Copy link
Contributor Author

@brandonrosage wow - this looks amazing 👯‍♂️

@willdoran
Copy link
Contributor Author

willdoran commented Nov 15, 2016

Test these changes by:

  • Create a new survey
  • Add to this survey a video embed field
  • Save survey
  • Add new post of the above survey type
  • Set the field to junk and attempt to save
  • Set field to youtube url of different types and save
  • Set field to vimeo url of different types and save
  • Attempt to view video on Post detail field
  • Attempt to edit
  • Change url and save

@jshorland
Copy link

jshorland commented Nov 15, 2016

  • Formatting for embed field instructions is off:
    screen shot 2016-11-15 at 4 23 26 pm
  • when not a proper youtube or vimeo url, it still saves:
    screen capture on 2016-11-15 at 16-26-43
  • After embedding a video, a bit of padding between link box and video preview would be nice
    screen capture on 2016-11-15 at 16-31-26

@willdoran
Copy link
Contributor Author

willdoran commented Nov 15, 2016

@jshorland All should be fixed by the latest

@jshorland
Copy link

Confirmed. Passes QA. lets get this shipped.

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

No branches or pull requests

4 participants