Moodle
  1. Moodle
  2. MDL-38012 META: Simplify moodle forms
  3. MDL-38049

Open page with advanced sections already hidden instead of hiding them with javascript after the page has loaded.

    Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.5
    • Fix Version/s: None
    • Component/s: Forms Library
    • Labels:
    • Testing Instructions:
      Hide
      1. Purge your cache
      2. Open the course settings page
      3. Make sure some sections are collapsed by default
        • Before the patch, the sections would be collapsed when the JS is entirely loaded, causing the page to flicker. This patch should fix this, by collapsing the sections by default, without flickering.
      Show
      Purge your cache Open the course settings page Make sure some sections are collapsed by default Before the patch, the sections would be collapsed when the JS is entirely loaded, causing the page to flicker. This patch should fix this, by collapsing the sections by default, without flickering.
    • Affected Branches:
      MOODLE_25_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-38049-master
    • Rank:
      47842

      Description

      The show more/show less sections of a form are initially all visible, and then get hidden. This causes a page reflow and is distracting.

      This page is an example of the issue:

      /admin/webservice/service.php?id=0

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          Hi, Damyon.

          I noticed this also, but I'm not sure we can achieve this while allowing the interface to function without JavaScript. Perhaps there is a way.

          Ruslan has probably thought about this, so perhaps he can comment.

          Show
          Michael de Raadt added a comment - Hi, Damyon. I noticed this also, but I'm not sure we can achieve this while allowing the interface to function without JavaScript. Perhaps there is a way. Ruslan has probably thought about this, so perhaps he can comment.
          Hide
          Damyon Wiese added a comment -

          Do you think there are any blockers for implementing this Ruslan?

          Show
          Damyon Wiese added a comment - Do you think there are any blockers for implementing this Ruslan?
          Hide
          Ruslan Kabalin added a comment -

          Hi Damyon, As Michael stated, we need to make it work properly without javascript (i.e. no advanced links displayed and all sections are expanded). Do you have a suggestion how to load page already with the right classes in place and make it useful for non-javascript?

          Show
          Ruslan Kabalin added a comment - Hi Damyon, As Michael stated, we need to make it work properly without javascript (i.e. no advanced links displayed and all sections are expanded). Do you have a suggestion how to load page already with the right classes in place and make it useful for non-javascript?
          Hide
          Michael de Raadt added a comment -

          After a week of using forms with the simplification, I think working on this issue should be a priority. Because we are now delaying the collapse of form sections until the page is completely loaded, it is quite noticeable on some pages and would be exacerbated by a slow connection. Also, the collapse seems to cause form elements to lose focus, which can be really annoying.

          Can we simply bring the JS to collapse the form forward, so that it is executed as soon as the form is present?

          Show
          Michael de Raadt added a comment - After a week of using forms with the simplification, I think working on this issue should be a priority. Because we are now delaying the collapse of form sections until the page is completely loaded, it is quite noticeable on some pages and would be exacerbated by a slow connection. Also, the collapse seems to cause form elements to lose focus, which can be really annoying. Can we simply bring the JS to collapse the form forward, so that it is executed as soon as the form is present?
          Hide
          Damyon Wiese added a comment -

          Without looking at the code again - we might be able to just add the collapsed classes in the html before we run the JS.

          Show
          Damyon Wiese added a comment - Without looking at the code again - we might be able to just add the collapsed classes in the html before we run the JS.
          Hide
          Michael de Raadt added a comment -

          I found this documentation for the .jsenabled class...

          http://docs.moodle.org/dev/JavaScript_guidelines#Content_that_should_only_be_visible_with_JavaScript_off

          Damyon pointed me to an example in the assignment module...

          /mod/assign/styles.css, lines 141-148
          .expandsummaryicon {
              cursor: pointer;
              display: none;
          }
          
          .jsenabled .expandsummaryicon {
              display: inline;
          }
          
          Show
          Michael de Raadt added a comment - I found this documentation for the .jsenabled class... http://docs.moodle.org/dev/JavaScript_guidelines#Content_that_should_only_be_visible_with_JavaScript_off Damyon pointed me to an example in the assignment module... /mod/assign/styles.css, lines 141-148 .expandsummaryicon { cursor: pointer; display: none; } .jsenabled .expandsummaryicon { display: inline; }
          Hide
          Ruslan Kabalin added a comment -

          OK, I will have a look what we can do with .jsenabled

          Show
          Ruslan Kabalin added a comment - OK, I will have a look what we can do with .jsenabled
          Hide
          Frédéric Massart added a comment -

          Hi Ruslan,

          I took the liberty of taking over this issue as it was still open and I was assigned to work on MDL-38012 related issues. I hope that was ok with you!

          Cheers,
          Fred

          Show
          Frédéric Massart added a comment - Hi Ruslan, I took the liberty of taking over this issue as it was still open and I was assigned to work on MDL-38012 related issues. I hope that was ok with you! Cheers, Fred
          Hide
          Ruslan Kabalin added a comment -

          That is fine Fred

          Show
          Ruslan Kabalin added a comment - That is fine Fred
          Hide
          Michael de Raadt added a comment -

          A peer review for you...

          [Y] Syntax
          [Y] Output
          [Y] Whitespace
          [-] Language
          [-] Databases
          [N] Testing
          [-] Security
          [-] Documentation
          [Y] Git
          [Y] Sanity check

          Well done for spotting the required change the to Behat test.

          I think the testing instructions could be expanded. I would suggest purging caches prior to starting. As there is a timing aspect involved, perhaps the tester could be informed about what to look for before they refresh the page. A refresh bypassing the browser cache should slow things down and make the loading and change easier to observe.

          Show
          Michael de Raadt added a comment - A peer review for you... [Y] Syntax [Y] Output [Y] Whitespace [-] Language [-] Databases [N] Testing [-] Security [-] Documentation [Y] Git [Y] Sanity check Well done for spotting the required change the to Behat test. I think the testing instructions could be expanded. I would suggest purging caches prior to starting. As there is a timing aspect involved, perhaps the tester could be informed about what to look for before they refresh the page. A refresh bypassing the browser cache should slow things down and make the loading and change easier to observe.
          Hide
          Frédéric Massart added a comment -

          Thanks Michael!

          Show
          Frédéric Massart added a comment - Thanks Michael!
          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
          Damyon Wiese added a comment -

          I did some extra testing to make sure that an editor and a file picker still worked when they were loaded initially hidden (and they did). The pages I used for testing are: Lesson settings and Quiz settings.

          Show
          Damyon Wiese added a comment - I did some extra testing to make sure that an editor and a file picker still worked when they were loaded initially hidden (and they did). The pages I used for testing are: Lesson settings and Quiz settings.
          Hide
          Damyon Wiese added a comment -

          Thanks Fred,

          This works much better now and the patch was good.

          Integrated to master.

          Show
          Damyon Wiese added a comment - Thanks Fred, This works much better now and the patch was good. Integrated to master.
          Hide
          Rajesh Taneja added a comment -

          Thanks Fred,

          Works Great. Sections are collapsed by default now.

          Show
          Rajesh Taneja added a comment - Thanks Fred, Works Great. Sections are collapsed by default now.
          Hide
          Damyon Wiese added a comment -

          This issue has been integrated upstream and is now available via git (and in some hours, via mirrors and downloads).

          Thanks for your contributions!

          Show
          Damyon Wiese added a comment - This issue has been integrated upstream and is now available via git (and in some hours, via mirrors and downloads). Thanks for your contributions!

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: