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

          Attachments

            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