Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-37194

Activity chooser is never displayed entirely on base theme

    Details

      Description

      With the base theme, the activity chooser never displays the full list of activity/resource even thought the window height is large enough.

      First screenshot: after fix
      Second screenshot: before fix

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            jerome Jérôme Mouneyrac added a comment -

            As it's javascript related, I sent it to you Andrew for peer-review.

            Show
            jerome Jérôme Mouneyrac added a comment - As it's javascript related, I sent it to you Andrew for peer-review.
            Hide
            dobedobedoh Andrew Nicols added a comment -

            Hi Jerome,

            This should really be done at the chooserdialogue level rather than the module chooser. Additionally, you need to update the CSS for

            .jsenabled .choosercontainer #chooseform .instruction, .jsenabled .choosercontainer #chooseform .typesummary

            The activity chooser was originally chosen at this size for aesthetic reasons.

            Adding Barbara as a watcher for her design opinion.

            Andrew

            Show
            dobedobedoh Andrew Nicols added a comment - Hi Jerome, This should really be done at the chooserdialogue level rather than the module chooser. Additionally, you need to update the CSS for .jsenabled .choosercontainer #chooseform .instruction, .jsenabled .choosercontainer #chooseform .typesummary The activity chooser was originally chosen at this size for aesthetic reasons. Adding Barbara as a watcher for her design opinion. Andrew
            Hide
            dobedobedoh Andrew Nicols added a comment -

            On second thoughts, I can see why you've kept the parameter change in the modchooser.js. Still needs the CSS change though, and I'd still appreciate barbara's thoughts here.

            Andrew

            Show
            dobedobedoh Andrew Nicols added a comment - On second thoughts, I can see why you've kept the parameter change in the modchooser.js. Still needs the CSS change though, and I'd still appreciate barbara's thoughts here. Andrew
            Hide
            jerome Jérôme Mouneyrac added a comment -

            Thanks for looking at it Andrew.

            In my opinion it's UX before aesthetic. I think users should not have to scroll when it's not necessary.

            Thanks for the CSS I have forgotten it in the patch.

            As you noticed I just increased the maximum size - it's currently fixed to a lower value. In my opinion having a maximum pixel size is wrong anyway. It already looks tiny on my computer, it will look even smaller on the futur ultra HD monitors/TVs. But this quick patch still being an improvement I thought I'll send it anyway

            About aesthetic - some personal taste:
            I would remove the top bar (title and cross). The user knows he's adding an activity, no need to take some space. In my opinion, it looks cleaner. The cross button is redundant with the Cancel button as the window is always on screen. The cross button has square corners which looks weird against the dialog rounded corners.

            I'll wait for Barbara opinions before doing any changes.

            Show
            jerome Jérôme Mouneyrac added a comment - Thanks for looking at it Andrew. In my opinion it's UX before aesthetic. I think users should not have to scroll when it's not necessary. Thanks for the CSS I have forgotten it in the patch. As you noticed I just increased the maximum size - it's currently fixed to a lower value. In my opinion having a maximum pixel size is wrong anyway. It already looks tiny on my computer, it will look even smaller on the futur ultra HD monitors/TVs. But this quick patch still being an improvement I thought I'll send it anyway About aesthetic - some personal taste: I would remove the top bar (title and cross). The user knows he's adding an activity, no need to take some space. In my opinion, it looks cleaner. The cross button is redundant with the Cancel button as the window is always on screen. The cross button has square corners which looks weird against the dialog rounded corners. I'll wait for Barbara opinions before doing any changes.
            Hide
            jerome Jérôme Mouneyrac added a comment -

            Changing the peer-review to Barbara so she can notice it.

            Show
            jerome Jérôme Mouneyrac added a comment - Changing the peer-review to Barbara so she can notice it.
            Hide
            jerome Jérôme Mouneyrac added a comment -

            I asked Barbara, she's ok. I'll check css you mention and I'll submit to integration. Thanks Andrew.

            Show
            jerome Jérôme Mouneyrac added a comment - I asked Barbara, she's ok. I'll check css you mention and I'll submit to integration. Thanks Andrew.
            Hide
            jerome Jérôme Mouneyrac added a comment -

            Submitting to integration. tested on IE8/9 and Chrome.

            Show
            jerome Jérôme Mouneyrac added a comment - Submitting to integration. tested on IE8/9 and Chrome.
            Hide
            stronk7 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
            stronk7 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
            poltawski Dan Poltawski added a comment -

            Adding Mary Evans here as theme component maintainer.

            Show
            poltawski Dan Poltawski added a comment - Adding Mary Evans here as theme component maintainer.
            Hide
            poltawski Dan Poltawski added a comment -

            Integrated to master, 24 and 23 thanks Jerome.

            Show
            poltawski Dan Poltawski added a comment - Integrated to master, 24 and 23 thanks Jerome.
            Hide
            lazydaisy Mary Evans added a comment - - edited

            Please do be careful with the CSS in Base theme as it affects ALL the CORE themes one way or another, so be sure to test them all.

            Show
            lazydaisy Mary Evans added a comment - - edited Please do be careful with the CSS in Base theme as it affects ALL the CORE themes one way or another, so be sure to test them all.
            Hide
            jerome Jérôme Mouneyrac added a comment -

            Thanks Mary. From reading your comment I read a "testing guide line" for the issue tester. Or did you find anything wrong?

            Note: I remember testing four/five themes and there were all good. The reason I didn't test all of the core theme is that I don't consider that this fix will impact them - The fix allows a bigger activity chooser window and the Moodle JS is in charge to resize it anyway.

            Show
            jerome Jérôme Mouneyrac added a comment - Thanks Mary. From reading your comment I read a "testing guide line" for the issue tester. Or did you find anything wrong? Note: I remember testing four/five themes and there were all good. The reason I didn't test all of the core theme is that I don't consider that this fix will impact them - The fix allows a bigger activity chooser window and the Moodle JS is in charge to resize it anyway.
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            As seen on the attached pic , activity chooser in 23 has empty space, is that a bad thing?
            Thanks

            Show
            ankit_frenz Ankit Agarwal added a comment - As seen on the attached pic , activity chooser in 23 has empty space, is that a bad thing? Thanks
            Hide
            jerome Jérôme Mouneyrac added a comment -

            For me the current state of the patch is a better UX than having a scrolling bar. As you noticed this grey space and thought it was worth a comment, I would suggest that you open an new issue for it Obviously it didn't bug me, but now that you are mentioning I think like you that it would be better to have a window that fit the activity list.

            From now I would pass this improvement issue and create a new improvement issue about the activity chooser not fitting the activity list (which was already the case before, but at least now we see the entire list).

            Show
            jerome Jérôme Mouneyrac added a comment - For me the current state of the patch is a better UX than having a scrolling bar. As you noticed this grey space and thought it was worth a comment, I would suggest that you open an new issue for it Obviously it didn't bug me, but now that you are mentioning I think like you that it would be better to have a window that fit the activity list. From now I would pass this improvement issue and create a new improvement issue about the activity chooser not fitting the activity list (which was already the case before, but at least now we see the entire list).
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            Passing based on the comments above, created and linked issue.
            Thanks

            Show
            ankit_frenz Ankit Agarwal added a comment - Passing based on the comments above, created and linked issue. Thanks
            Hide
            poltawski Dan Poltawski added a comment -

            Hurray! We did it! Thanks to all the reporters, testers, user and watchers for a bumper week of Moodling!

            Show
            poltawski Dan Poltawski added a comment - Hurray! We did it! Thanks to all the reporters, testers, user and watchers for a bumper week of Moodling!

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  11/Mar/13