Moodle
  1. Moodle
  2. MDL-42111

Youtube Repository Video Preview is Cut Off

    Details

    • Testing Instructions:
      Hide
      1. Ensure YouTube repository is enabled on site.
      2. Ensure the site theme (or course theme) is one of the affected themes (Clean, Formal White, Boxxie, splash and Brick themes).
      3. Access the text editor in any course assignment, label, etc.
      4. Click "Insert Moodle Media".
      5. Click "Find or upload a sound, video or applet..."
      6. Click on YouTube repository and search for literally any YouTube video.
      7. Select the desired video; click "Select this file".
      8. Review the video in the "Insert Moodle Media" box.
      9. For the affected themes, the video will be cut off and not display the entire video thumbnail.

      Expected result: The full width of the video is viewable in the preview window.

      Show
      Ensure YouTube repository is enabled on site. Ensure the site theme (or course theme) is one of the affected themes (Clean, Formal White, Boxxie, splash and Brick themes). Access the text editor in any course assignment, label, etc. Click "Insert Moodle Media". Click "Find or upload a sound, video or applet..." Click on YouTube repository and search for literally any YouTube video. Select the desired video; click "Select this file". Review the video in the "Insert Moodle Media" box. For the affected themes, the video will be cut off and not display the entire video thumbnail. Expected result: The full width of the video is viewable in the preview window.
    • Affected Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE
    • Pull from Repository:
    • Pull 2.4 Branch:
      MDL-42111_m24
    • Pull 2.5 Branch:
      MDL-42111_m25
    • Pull Master Branch:
    • Story Points:
      8
    • Rank:
      53346
    • Sprint:
      FRONTEND Sprint 6

      Description

      When using "Insert Moodle Media" to add a Youtube video (from the Youtube repository) to an HTML editor, the preview of the video is cut off and displays only as a very narrow vertical strip. Please see the attached screenshot.

      Note: this seems to be a problem only in the Formal White, Boxie, and Brick themes. The following themes display the video preview correctly: Anomaly, Arialist, Binarius, FormFactor, Fusion, Leatherbound and Magazine.

      Steps to replicate:

      1. Ensure YouTube repository is enabled on site.
      2. Ensure the site theme (or course theme) is one of the affected themes.
      3. Access the text editor in any course assignment, label, etc.
      4. Click "Insert Moodle Media".
      5. Click "Find or upload a sound, video or applet..."
      6. Click on YouTube repository and search for literally any YouTube video.
      7. Select the desired video; click "Select this file".
      8. Review the video in the "Insert Moodle Media" box.
      9. For the affected themes, the video will be cut off and not display the entire video thumbnail.

      Expected result: The full width of the video is viewable in the preview window.

      Actual result: The video preview shows only a very narrow vertical strip.

      1. boxxie.png
        319 kB
      2. clean.png
        261 kB
      3. youtubepreview.png
        43 kB

        Issue Links

          Activity

          Hide
          Andrew Nicols added a comment -

          [Y] Syntax
          [Y] Whitespace
          [N] Output
          [-] Language
          [-] Databases
          [Y] Testing (instructions and automated tests)
          [-] Security
          [-] Documentation
          [N] Git
          [-] Third party code
          [Y] Sanity check

          Output
          • The video doesn't fit nicely in the boxxie theme ()
          • I'm seeing some other issues on the clean theme too which it may be appropriate to address in this issue. ()
          Git

          I feel the commit message could be a little clearer and state TinyMCE. Only a niggle though so feel free to ignore

          Otherwise, feel free to submit for integration

          Show
          Andrew Nicols added a comment - [Y] Syntax [Y] Whitespace [N] Output [-] Language [-] Databases [Y] Testing (instructions and automated tests) [-] Security [-] Documentation [N] Git [-] Third party code [Y] Sanity check Output The video doesn't fit nicely in the boxxie theme ( ) I'm seeing some other issues on the clean theme too which it may be appropriate to address in this issue. ( ) Git I feel the commit message could be a little clearer and state TinyMCE. Only a niggle though so feel free to ignore Otherwise, feel free to submit for integration
          Hide
          Rossiani Wijaya added a comment -

          updating patch to remove the unnecessary srollbars.

          Andrew, could you review this again?

          Thanks.

          Show
          Rossiani Wijaya added a comment - updating patch to remove the unnecessary srollbars. Andrew, could you review this again? Thanks.
          Hide
          Andrew Nicols added a comment -

          Hi Rossiani Wijaya,

          The theme's you've done look fine to me now.

          It seems that a lot of the themes are inconsistent still sadly, but they're not affected by the original issue and they don't prevent use of the viewer as before.

          I suggest we submit this issue for integration now, and raise a new issue to review all of the themes (perhaps we can automate this - Marina Glancy was working on something to screenshot from behat instructions IIRC). I had a quick look at a few random themes which were affected:

          • scrollbars present:
            • formal_white;
          • not centred vertically:
            • afterburner;
            • binarius;
            • overlay;
            • magazine.

          So feel free to submit for integration when ready, but please raise an issue for the remaining themes.

          Cheers,

          Andrew

          Show
          Andrew Nicols added a comment - Hi Rossiani Wijaya , The theme's you've done look fine to me now. It seems that a lot of the themes are inconsistent still sadly, but they're not affected by the original issue and they don't prevent use of the viewer as before. I suggest we submit this issue for integration now, and raise a new issue to review all of the themes (perhaps we can automate this - Marina Glancy was working on something to screenshot from behat instructions IIRC). I had a quick look at a few random themes which were affected: scrollbars present: formal_white; not centred vertically: afterburner; binarius; overlay; magazine. So feel free to submit for integration when ready, but please raise an issue for the remaining themes. Cheers, Andrew
          Hide
          Marina Glancy added a comment -

          Hi, this is the tool to save screenshots https://github.com/marinaglancy/moodle-tool_behatui . Sorry, I have not used it for a while, hopefully it will work with 2.6 code

          Show
          Marina Glancy added a comment - Hi, this is the tool to save screenshots https://github.com/marinaglancy/moodle-tool_behatui . Sorry, I have not used it for a while, hopefully it will work with 2.6 code
          Hide
          Rossiani Wijaya added a comment -

          Thanks Andrew and Marina for the save screenshot tool.

          I created MDL-42444 to fix the preview display in other themes.

          Submitting this for integration review.

          Show
          Rossiani Wijaya added a comment - Thanks Andrew and Marina for the save screenshot tool. I created MDL-42444 to fix the preview display in other themes. Submitting this for integration review.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Dan Poltawski added a comment -

          Integrated to master, 25 and 24 - thanks Rosie.

          Please watch out for trailing whitespace.

          Show
          Dan Poltawski added a comment - Integrated to master, 25 and 24 - thanks Rosie. Please watch out for trailing whitespace.
          Hide
          Petr Škoda added a comment -

          works fine for me, thanks

          Show
          Petr Škoda added a comment - works fine for me, thanks
          Hide
          Damyon Wiese added a comment -

          Here lies 52 bugs.
          All fixed or swept under a rug.
          If they come back one day,
          To our dismay,
          We all will feel quite un-smug.

          Thanks for the reporting/fixing/testing on this issue. It has been sent upstream.

          Show
          Damyon Wiese added a comment - Here lies 52 bugs. All fixed or swept under a rug. If they come back one day, To our dismay, We all will feel quite un-smug. Thanks for the reporting/fixing/testing on this issue. It has been sent upstream.

            People

            • Votes:
              1 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Agile