search

rethinkdb / docs

RethinkDB documentation
http://rethinkdb.com/docs
Apache License 2.0
117 stars 167 forks source link

Youtube link in index.html (replacing video tag over img) #1311

Open Sudhee-bsp opened 3 years ago

Sudhee-bsp commented 3 years ago

Describing the bug A short intro video which is listed under the RethinkDB in two minutes isn't playing on the index.html page.

To Reproduce Steps to reproduce the behavior:

  1. Go to https://rethinkdb.com/docs
  2. Click on the play symbol. (It's an image)

Expected behavior Clicking on the play symbol should actually play the video. That seems to be good for any user who views the docs (also distracting redirecting to youtube website may not be a good thing)

Additional context Efforts to change the above thing may not be a big change, but it's simply a dumb thing putting an image over there with a play symbol. Putting a hyperlink may also seem fine rather now it is.

clovergaze commented 3 years ago

Describing the bug A short intro video which is listed under the RethinkDB in two minutes isn't playing on the index.html page.

To Reproduce Steps to reproduce the behavior:

  1. Go to https://rethinkdb.com/docs
  2. Click on the play symbol. (It's an image)

Expected behavior Clicking on the play symbol should actually play the video. That seems to be good for any user who views the docs (also distracting redirecting to youtube website may not be a good thing)

Additional context Efforts to change the above thing may not be a big change, but it's simply a dumb thing putting an image over there with a play symbol. Putting a hyperlink may also seem fine rather now it is.

I can confirm that the video is not displayed correctly on the current index page of the online documentation.

Clicking the image should actually open the video on the same page (no redirection to YouTube) in a modal div layer that contains the embedded YouTube video as iframe (that's generated via Jekyll, I guess).

It does however work locally for me as expected when I generate the documentation via rake.

Something probably went wrong when the online documentation was generated and my guess would be that it just needs to be regenerated to fix this problem.

Sudhee-bsp commented 3 years ago

I see, Great, for working when locally generated via rake, And Now, does it need a PR to fix this issue? Or current version re-generation may fix the issue.

clovergaze commented 3 years ago

I see, Great, for working when locally generated via rake, And Now, does it need a PR to fix this issue? Or current version re-generation may fix the issue.

Maybe @srh can initiate a rebuild and redeploy of the documentation (no sure about the build pipelines there) and than we see whether it works again or not. I used the current master branch for testing this locally, it should work the same for the online version.

I think we should stick to the current Ruby/Jekyll method of generating this and only use an embedded YouTube iframe of the video, if that really does not work for some reason.

srh commented 3 years ago

Thank you for the report. The website gets redeployed whenever we update the master branch (of www? of docs? I forget). So I merged one of the dependabot PR's: https://github.com/rethinkdb/www/pull/300

and if that doesn't work, we have something to fix.

clovergaze commented 3 years ago

The video still does not play. Maybe the rebuild is triggered by changes in the master branch of the docs repo?

Sudhee-bsp commented 3 years ago

@clovergaze May be you can create a PR, with your local working code, via rake. Does it work like that?

clovergaze commented 3 years ago

@clovergaze May be you can create a PR, with your local working code, via rake. Does it work like that?

I used the most current code base to test this, I did not change anything.. or did you mean as a kind of dummy PR to trigger the build process for the docs repo?

Sudhee-bsp commented 3 years ago

yeah, a dummy PR to trigger the build process again and, you can assign yourself this issue and give a PR, and @srh may merge after approving. so that, this way it may work. 🙂

clovergaze commented 3 years ago

@Sudhee-bsp @srh

yeah, a dummy PR to trigger the build process again and, you can assign yourself this issue and give a PR, and @srh may merge after approving. so that, this way it may work. 🙂

I created this simple pull request: #1312

Not sure whether or not this triggers the rebuild process of the webpage as it is just a minor change in the README file, but we should try.

Judging from what is written in main.yml, any change to the master branch should trigger a redeploy.

clovergaze commented 3 years ago

The video still does not open as expected, so that did not fix it.. :(