Moodle
  1. Moodle
  2. MDL-38850

Horizontal scroll bar not present in Lesson

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.4.3, 2.5.2
    • Fix Version/s: 2.4.7, 2.5.3
    • Component/s: Lesson
    • Labels:
    • Testing Instructions:
      Hide

      1. Add a Lesson.
      2. Add a content page to it.
      3. Insert a large image in the page content and save it.
      4. Go to the "Preview" tab to display it.

      You'll notice that the horizontal scroll is not present at the bottom of the image.

      5. Go to the "Edit" tab and click on the lesson.

      You'll notice that as the table is very wide due to the large image, the horizontal bar is not present.

      Show
      1. Add a Lesson. 2. Add a content page to it. 3. Insert a large image in the page content and save it. 4. Go to the "Preview" tab to display it. You'll notice that the horizontal scroll is not present at the bottom of the image. 5. Go to the "Edit" tab and click on the lesson. You'll notice that as the table is very wide due to the large image, the horizontal bar is not present.
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      Earlier, we reported the same problem in Book (MDL-38794) – a horizontal scroll bar also is not present in a Lesson page where the content on that page is very wide. A full explanation is available in MDL-38794.

        Gliffy Diagrams

        1. patch.txt
          1 kB
          ayush lodhi
        1. lesson_with_horizontal_scroll.jpg
          145 kB
        2. lesson_without_horizontal_scroll.jpg
          127 kB

          Issue Links

            Activity

            Hide
            Michael de Raadt added a comment -

            Thanks for reporting that, also.

            Noting that there was a patch on the linked issue that could probably be adapted to the CSS in lesson.

            Show
            Michael de Raadt added a comment - Thanks for reporting that, also. Noting that there was a patch on the linked issue that could probably be adapted to the CSS in lesson.
            Hide
            ayush lodhi added a comment -

            check this patch, horizontal scroll is now present in the lesson!

            Show
            ayush lodhi added a comment - check this patch, horizontal scroll is now present in the lesson!
            Hide
            Puneet Kala added a comment -

            Is this Issue fixed? Or I should start working on that?

            Show
            Puneet Kala added a comment - Is this Issue fixed? Or I should start working on that?
            Hide
            Prateek Sachan added a comment - - edited

            Hi Michael,

            As I mentioned in MDL-38794, I've provided the fix for the bug for Lesson mod for both the branches:

            master: https://github.com/prateeksachan/moodle/compare/MDL-38850
            MOODLE_24_STABLE : https://github.com/prateeksachan/moodle/compare/MOODLE_24_STABLE...MDL-38850_24

            Show
            Prateek Sachan added a comment - - edited Hi Michael, As I mentioned in MDL-38794 , I've provided the fix for the bug for Lesson mod for both the branches: master: https://github.com/prateeksachan/moodle/compare/MDL-38850 MOODLE_24_STABLE : https://github.com/prateeksachan/moodle/compare/MOODLE_24_STABLE...MDL-38850_24
            Hide
            kaushal singh added a comment - - edited

            Hello Michael,
            I have fixed bug 'MDL-38850 Horizontal scroll bar not present in Lesson'.
            Here is the commit:
            MDL-38850 Horizontal scroll bar added in Lesson
            https://github.com/kaushalsingh11/moodle/commit/bd5588be3eef6e6ed3d67cf87958b5c4eb6bc5e3

            Show
            kaushal singh added a comment - - edited Hello Michael, I have fixed bug ' MDL-38850 Horizontal scroll bar not present in Lesson'. Here is the commit: MDL-38850 Horizontal scroll bar added in Lesson https://github.com/kaushalsingh11/moodle/commit/bd5588be3eef6e6ed3d67cf87958b5c4eb6bc5e3
            Hide
            Aparup Banerjee added a comment -

            The fix with the API option at seems to be the way (https://github.com/prateeksachan/moodle/compare/MDL-38850).

            However i think it would be easier if all the similar situations were handled in MDL-38794 at the same time. This would be easier too to write behaviour (behat) tests for it.

            Show
            Aparup Banerjee added a comment - The fix with the API option at seems to be the way ( https://github.com/prateeksachan/moodle/compare/MDL-38850 ). However i think it would be easier if all the similar situations were handled in MDL-38794 at the same time. This would be easier too to write behaviour (behat) tests for it.
            Hide
            Prateek Sachan added a comment - - edited

            Isn't the fix above correct? (https://github.com/prateeksachan/moodle/compare/MDL-38850)

            The above patch I gave, fixed the issue in "Preview" of Lesson.
            However, the "Edit" tab was still having problems of horizontal scrolling.

            For resolving the problem on "Edit" tab, I had to modify another file.

            I've updated the commit.

            Show
            Prateek Sachan added a comment - - edited Isn't the fix above correct? ( https://github.com/prateeksachan/moodle/compare/MDL-38850 ) The above patch I gave, fixed the issue in "Preview" of Lesson. However, the "Edit" tab was still having problems of horizontal scrolling. For resolving the problem on "Edit" tab, I had to modify another file. I've updated the commit.
            Hide
            Anup Kumar added a comment -

            https://github.com/anupraaj/moodle/commit/a07e514a5af93af6d22a12bfcb401d49ebd5ed1e

            Here is problem resolved. I am assuming that only horizontal scroll has to be activated.

            Show
            Anup Kumar added a comment - https://github.com/anupraaj/moodle/commit/a07e514a5af93af6d22a12bfcb401d49ebd5ed1e Here is problem resolved. I am assuming that only horizontal scroll has to be activated.
            Hide
            Rossiani Wijaya added a comment -

            This issue was assigned to me automatically, however I will not be able to work on this issue in the immediate future. In order to create a truer sense of the state of this issue and to allow other developers to have chance to become involved, I am removing myself as the assignee of this issue.

            Show
            Rossiani Wijaya added a comment - This issue was assigned to me automatically, however I will not be able to work on this issue in the immediate future. In order to create a truer sense of the state of this issue and to allow other developers to have chance to become involved, I am removing myself as the assignee of this issue.
            Hide
            Prateek Sachan added a comment -

            Clicked on "Start peer review" by mistake. Requesting peer-review again.
            Sorry!

            Show
            Prateek Sachan added a comment - Clicked on "Start peer review" by mistake. Requesting peer-review again. Sorry!
            Hide
            Rossiani Wijaya added a comment -

            Hi Prateek,

            Thank you for working on this issue.

            Could you please add the scroll bar to the report page as well?

            Other than that, the patch looks great.

            Rosie

            Show
            Rossiani Wijaya added a comment - Hi Prateek, Thank you for working on this issue. Could you please add the scroll bar to the report page as well? Other than that, the patch looks great. Rosie
            Hide
            Prateek Sachan added a comment -

            Hi Rossiani,

            I've updated the commit and included the patch for report page as well.
            Thanks for pointing that out.

            Show
            Prateek Sachan added a comment - Hi Rossiani, I've updated the commit and included the patch for report page as well. Thanks for pointing that out.
            Hide
            Rossiani Wijaya added a comment -

            Hi Prateek,

            Thanks for working on this. The patch looks good.
            However before submitting this for integration review, could you fixed the coding standard for locallib file (including the inline comment)?

            Coding styling guideline is available in: http://docs.moodle.org/dev/Coding_style.

            Once this is fixed. It is good for integration.

            Rosie.

            Show
            Rossiani Wijaya added a comment - Hi Prateek, Thanks for working on this. The patch looks good. However before submitting this for integration review, could you fixed the coding standard for locallib file (including the inline comment)? Coding styling guideline is available in: http://docs.moodle.org/dev/Coding_style . Once this is fixed. It is good for integration. Rosie.
            Hide
            Prateek Sachan added a comment -

            Hi Rossiani Wijaya,

            I've updated the commits.

            Thanks.

            Show
            Prateek Sachan added a comment - Hi Rossiani Wijaya , I've updated the commits. Thanks.
            Hide
            Rossiani Wijaya added a comment -

            Hi Prateek,

            Thank you for updating the patch. However, the changes you made for lesson/locallib.php is not following the coding standard we have for Moodle. Could you please re-update it and use the standard in http://docs.moodle.org/dev/Coding_style#Wrapping_function_declarations.

            Thanks.

            Show
            Rossiani Wijaya added a comment - Hi Prateek, Thank you for updating the patch. However, the changes you made for lesson/locallib.php is not following the coding standard we have for Moodle. Could you please re-update it and use the standard in http://docs.moodle.org/dev/Coding_style#Wrapping_function_declarations . Thanks.
            Hide
            Prateek Sachan added a comment -

            Hi Rossiani,

            I've again updated the commits and checked them with codechecker and moodlecheck plugins.

            Thanks.

            Show
            Prateek Sachan added a comment - Hi Rossiani, I've again updated the commits and checked them with codechecker and moodlecheck plugins. Thanks.
            Hide
            Rossiani Wijaya added a comment -

            Hi Prateek,

            I updated the spacing on your patch and kept your name as the author.

            Thanks for working on this issue.

            Submitting for integration review.

            Show
            Rossiani Wijaya added a comment - Hi Prateek, I updated the spacing on your patch and kept your name as the author. Thanks for working on this issue. Submitting for integration review.
            Hide
            Prateek Sachan added a comment -

            Thanks Rossiani.

            Show
            Prateek Sachan added a comment - Thanks Rossiani.
            Hide
            Damyon Wiese added a comment -

            Thanks Prateek,

            The patch looks good to me.

            Note - for tester, in clean theme the image will likely be scaled rather than shown at the proper size with a scrollbar. Not sure why - but don't count that as a failure (if it's not intentional, it's a separate issue at least).

            Integrated to 24, 25 and master.

            Show
            Damyon Wiese added a comment - Thanks Prateek, The patch looks good to me. Note - for tester, in clean theme the image will likely be scaled rather than shown at the proper size with a scrollbar. Not sure why - but don't count that as a failure (if it's not intentional, it's a separate issue at least). Integrated to 24, 25 and master.
            Hide
            Rajesh Taneja added a comment -

            Thanks for fixing this Prateek,

            Works fine for me... Passing...

            Show
            Rajesh Taneja added a comment - Thanks for fixing this Prateek, Works fine for me... Passing...
            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:
                0 Vote for this issue
                Watchers:
                12 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: