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:
    • Rank:
      48932

      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.

      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: