Moodle
  1. Moodle
  2. MDL-30641

Filepicker lists folders with <i><u>...</u></i> when in 'View as list' mode

    Details

    • Testing Instructions:
      Hide

      Requirements

      These instructions require a repository with a sub folder. I used the 'Server files' repository, but any will do.

      Instructions

      • open a course, turn editing mode on, and add a page;
      • under the description, choose the 'moodle media' button;
      • choose 'Find or upload a sound, video or applet...';
      • when the filepicker opens, choose 'View as list'; and
      • navigate to a repository with folders within it (e.g. Server Files).

      Expected Result

      A list of folder names

      Actual Result

      A list folder names wrapped in <i><u> tags

      Further instructions

      • now select 'View as icons' again; and
      • mouse over one of the folders.

      Expected Result

      The folders should not be italicised or underlined
      The anchor title is the filename

      Actual Result

      The folders are italicised and underlined
      The anchor title is set to the <i><u> text

      Show
      Requirements These instructions require a repository with a sub folder. I used the 'Server files' repository, but any will do. Instructions open a course, turn editing mode on, and add a page; under the description, choose the 'moodle media' button; choose 'Find or upload a sound, video or applet...'; when the filepicker opens, choose 'View as list'; and navigate to a repository with folders within it (e.g. Server Files). Expected Result A list of folder names Actual Result A list folder names wrapped in <i><u> tags Further instructions now select 'View as icons' again; and mouse over one of the folders. Expected Result The folders should not be italicised or underlined The anchor title is the filename Actual Result The folders are italicised and underlined The anchor title is set to the <i><u> text
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-30641-master-1

      Description

      When using the file picker and viewing items using the 'View as list' option, any folders are displayed as
      <i><u>TITLE</u></i>

      After viewing as 'View as list', changing back to 'View as icons' gives similar results for the anchor titles, and the filename of the folders

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Andrew Nicols added a comment -

            Also noticed that when using 'View as Icons', the anchor title contains the same tags

            Show
            Andrew Nicols added a comment - Also noticed that when using 'View as Icons', the anchor title contains the same tags
            Hide
            Andrew Nicols added a comment -

            I'm not convinced of the benefits of italicising, or underlining the folders at all.

            The patch I've attached removes the code which does so and thus resolves the problem.

            If this is desired, then appropriate css tags should be added and it should really be done with CSS allowing theme designers to disagree amongst one another

            Show
            Andrew Nicols added a comment - I'm not convinced of the benefits of italicising, or underlining the folders at all. The patch I've attached removes the code which does so and thus resolves the problem. If this is desired, then appropriate css tags should be added and it should really be done with CSS allowing theme designers to disagree amongst one another
            Hide
            Andrew Nicols added a comment -

            This patch cherry-picks cleanly to Moodle 2.1

            Show
            Andrew Nicols added a comment - This patch cherry-picks cleanly to Moodle 2.1
            Hide
            Andrew Nicols added a comment -

            I've also pushed another commit MDL-30641-master-2 which modifies the way that the file picker presents the elements

            For the icons format, it uses the YUI3 Node.create function rather than document.createElement which means that it's no longer necessary to grab the element name by creating an id, and manipulating it as it's possible to add the listeners directly to the YUI3 Node object. I've also modified the HTML tree to wrap the entire file/folder element in one anchor, rather than having multiple anchors each with their own listener.

            For the list format, I've simply added a class of folder/file to the tree node so that it should be possble to use CSS such as:

            div.fp-tree-panel table.folder {
              font-weight: bold;
            }
             
            div.fp-tree-panel table.file {
              font-style: italic;
            }
            

            I suspect that this change is too large for the 2.1 and 2.2 series, but it might be considered for master

            Show
            Andrew Nicols added a comment - I've also pushed another commit MDL-30641 -master-2 which modifies the way that the file picker presents the elements For the icons format, it uses the YUI3 Node.create function rather than document.createElement which means that it's no longer necessary to grab the element name by creating an id, and manipulating it as it's possible to add the listeners directly to the YUI3 Node object. I've also modified the HTML tree to wrap the entire file/folder element in one anchor, rather than having multiple anchors each with their own listener. For the list format, I've simply added a class of folder/file to the tree node so that it should be possble to use CSS such as: div.fp-tree-panel table.folder { font-weight: bold; }   div.fp-tree-panel table.file { font-style: italic; } I suspect that this change is too large for the 2.1 and 2.2 series, but it might be considered for master
            Hide
            Sam Hemelryk added a comment -

            Hi Andrew,
            Normally Dongsheng would peer-review filepicker issues, however he is away at the moment so I'll pick this up.

            Cheers
            Sam

            Show
            Sam Hemelryk added a comment - Hi Andrew, Normally Dongsheng would peer-review filepicker issues, however he is away at the moment so I'll pick this up. Cheers Sam
            Hide
            Sam Hemelryk added a comment -

            Thanks Andrew, changes look spot on!
            Feel free to put up for integration when you're ready.

            Show
            Sam Hemelryk added a comment - Thanks Andrew, changes look spot on! Feel free to put up for integration when you're ready.
            Hide
            Sam Hemelryk added a comment -

            I forgot to mention, I also had a quick look at the MDL-30641-master-2 branch.
            While I didn't review it properly the changes that you are making there look good, and certainly its a brilliant idea.
            I think that certainly it should be split off onto a separate issue. It is an improvement but it would get a +1 from me for master & MOODLE_22_STABLE.

            Cheers
            Sam

            Show
            Sam Hemelryk added a comment - I forgot to mention, I also had a quick look at the MDL-30641 -master-2 branch. While I didn't review it properly the changes that you are making there look good, and certainly its a brilliant idea. I think that certainly it should be split off onto a separate issue. It is an improvement but it would get a +1 from me for master & MOODLE_22_STABLE. Cheers Sam
            Hide
            Andrew Nicols added a comment -

            Just to confirm, this should cherry-pick cleanly to:

            • master
            • MOODLE_22_STABLE
            • MOODLE_21_STABLE
            Show
            Andrew Nicols added a comment - Just to confirm, this should cherry-pick cleanly to: master MOODLE_22_STABLE MOODLE_21_STABLE
            Hide
            Aparup Banerjee added a comment -

            Thanks , this has been integrated into master and picked into 22 and 21 branches

            tester: remember to purge caches

            Show
            Aparup Banerjee added a comment - Thanks , this has been integrated into master and picked into 22 and 21 branches tester: remember to purge caches
            Hide
            Ankit Agarwal added a comment -

            this is working great!

            Show
            Ankit Agarwal added a comment - this is working great!
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Whoever decided one week was worth 14 days had really one bad idea. Anyway, the nightmare is over, so thanks for your, once again, amazing contributions. Many, many thanks!

            Now... disconnect, relax and enjoy the next days, yay!

            Closing...ciao

            Show
            Eloy Lafuente (stronk7) added a comment - Whoever decided one week was worth 14 days had really one bad idea. Anyway, the nightmare is over, so thanks for your, once again, amazing contributions. Many, many thanks! Now... disconnect, relax and enjoy the next days, yay! Closing...ciao

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: