Create a Javascript Module Chooser

This commit will work best when MDL-30627 is also applied

Hi, Andrew.

Could you please elaborate on this?

Hi Michael,

There's a detailed explanation of this functionality in the parent issue (MDL-30615) including a full functionality listing, link to discussion in the forum, and screenshot.

Setting Martin as peer reviewer so this can be considered for an upcoming major release.

How stable is this looking? Could it be really stable this week on top of the other YUI3-based course stuff?

We've had this in production since December 22nd, and developed it back in November 2011.

We've made a few modifications in that time, and also more recently largely due to the course ajax changes we've also been working on. There are also some minor bug fixes and cosmetic improvements (e.g. preventing the flashing of the chooser on page load).

At present this just follows the useajax setting in the user profile and not a specific setting for the module chooser.

MDL-32707 may cause some issues until it's integrated - the modchooser has a tendency to show behind youtube videos without this setting.

Seems like its broken with javascript disabled now?

Sorry about that Dan - I'd forgotten to add a CSS class to hide the link.

Have pushed an update to the CSS to fix this.

Andrew

Ahem - another minor correction

I've added the user preference as per your request. Haven't had a chance to get the right text - snowed under today.

We now add an additional setting, only shown when editing is enabled, to the navigation tree. The URL for this is in course/view.php (same as Turn editing on/off). It's also only shown if javascript is enabled in the browser and it applies the same rules as the module chooser being shown at all.

The module chooser javascript also adds an onclick event to toggle the menu item without a page reload.

The text on the toggle could do with being short really - they're onto two lines in the standard theme as is which doesn't fill me with a warm fuzzy feeling.

Oh, meant to say that I've also included the patches for MDL-32873 and MDL-32837 which relate to MDL-31215 and others.

And haven't had a chance to get the help links in place that Martin was talking about in devchat this morning.

Hi Andrew,

I'm just working on help texts for the module chooser - MDL-32925.

Re. help links, in the google doc https://docs.google.com/document/d/1KTZUWRJEqLdBKTInmrY4USliyZZD93lE4pWGBAjxG5w/edit I've listed help links used previously. You'll notice all the activity links are of the form mod/activityname/index and all the resource links are of the form mod/resourcename/view. Please feel free to change either/both, as it's very easy to create redirects in the wiki.

Modified to accept double clicks

Re. links to pages in the docs, how about having an i icon with a 'More help' link, as done elsewhere (see attachment).

I've added the help link as discussed.
I've pushed a new version which also has the current iterations of the language strings as an additional commit to give an idea as to how it will appear.

Some other things to note:

• the help link always opens in the same window. I think that we should consider modifying doc_link to take an option to force a new window
• I've made the width of the box fractionally wider to fully accommodate all of the help – the bottom line or two was being hidden

Corrected a minor variable naming bug.

Re. the 'More help' link opening in a new window, there is a setting doctonewwindow which controls whether 'Moodle Docs for this page' links open in a new window, and 'More help' links too.

The doctonewwindow setting is a global setting though. I think that it may be worthwhile to always open docs in new windows in certain contexts.

Updated for latest integration/master.git

I've added a snapshot of the new texts from yesterday so you can see how they look, but I've not updated with the final versions yet.

Thanks Andrew, the help texts will hopefully be finished tomorrow.

Hi,

Just been trying the latest. I think it's mostly great! The enable/disable thing in the course admin menu works well.

Some final things to fix IMO:

1) See Helen's comment above about the docs links, they need to go to /mod/xxxx/view not /mod/xxxx/
1) Yes, the moodle docs link here should always open in a new page, IMO. Could you just hardcode $CFG->doctonewwindow = true; before calling the help function?
2) I think it would be worth putting in small headings to clearly define the two lists in the dialogue for Resources and Activities. Those were a huge part of the old interface and we should retain that clarity.

Cheers,
Martin

Re. Martin's comment

1) See Helen's comment above about the docs links, they need to go to /mod/xxxx/view not /mod/xxxx/

I've just checked the de, fr, ja and es wikis and found virtually no redirects from mod/activityname/index or mod/resourcename/view. Thus I think it's a good time to implement modulename_link strings of the form mod/xxxx i.e. mod/assignment, mod/chat, ... mod/folder, mod/imscp, mod/lti etc as you have done Andrew (so no change necessary)

I'll make sure they all redirect to the correct pages in the en docs and other languages can follow suit.

Helen if you do that then remember to ALSO do mod/xxxx/view in those other lanaguages, because that is where the main page for those modules in Moodle is sending them.

Ultimately, Andrew I think it's better to change the code to mod/xxxxx/view after all.

Best for 3rd party modules too.

Hi Andrew, please scrub my previous comment and let's have all docs links of the form mod/xxxx/view. (Martin has finally managed to convince me!)

1: Will change these when Helen has finished the strings
2: This is actually not as easy as expected. Because YUI dialogues /copy/ the target rather than wrap it. The doc_link() function adds an onclick listener to the link to open it in a popup. Alas, YUI doesn't copy the onclick so it doesn't work. I'll add a fix to the initialiser instead.
3: Have removed the separator and added a title instead (both together looked wrong) - http://dumping.andrewrn.co.uk/modchooser.png

Just setting the status on this so it appears in my list

Added all language strings (except book) too

Apologies - corrected the diff to show both commits. Here they are separate:
https://git.luns.net.uk/moodle.git/commitdiff/5008917b636343bbc2207726f7a3609fef894a92 - chooser
https://git.luns.net.uk/moodle.git/commitdiff/6fa31373dbce7de3c02c4b7de97ff9509cef1629 - new language strings

For the record, I chatted with Andrew and asked him to move the choser to the right, like the menus - I think it helps distinguish between content and actions and also to convert to html_writer in some bits of html.

Updated to move to the right as per your request. I agree, it does look better.

It would also feel less jarring when switching between modchooser and dropdowns if they were the same height I think.

OK - congratulations! I've integrated it now.

Some notes, to be followed with issues:

• Functionality wise that enable/disable dialogue seems OK, but I think that is a horrible bit of jargon! For me its way too prominently placed on edit menu too. But others have OK'ed it. I really think we must sort out that title though.
• I've seen the popup appear in the wrong place a few times - need to look at that (I will send a screenshot in a second). Unlike the quiz one you can't move it either.
• Forum help text seems to scroll
• Glossary help text seems to scroll
• We need a nice simple CSS effect on the links on the course page to make them look like buttons. I experimented with this and I think it makes it look much better.
• We need to look at the CSS of the popup box to make it nicer, its actually less 'pretty' than the quiz one (not that i'm saying the quiz one is pretty). E.g. the top bar text is not centred.
• I did some minor tweaks to use html_writer and took away 'personlisation' from the lang string (plus 'a new' from 'Add a new resource or activity'

We need testing instructions for this

Attaching a screenshot. The dialog is too tall for my screen. This is on a 13" laptop with a screen resolution of 1366 x 768. Its not huge but its not tiny.

The scroll bar on the page doesnt help. It moves the page behind the dialog but the dialog stays where it is.

Grabbing the title bar of the dialog doesnt do anything. It doesnt seem to be possible move the dialog at all. If I could drag it up I could at least do that to bring the buttons into view. Although this would still be broken for anyone with a smaller screen than me.

I can just access the buttons at the bottom but I had to guess which button was which.

Added some basic testing instructions.

Things to fix:

1) Change "Disable module chooser dialogue" to "Activity chooser on" and "Activity chooser off".
2) Whole dialog must fit on the window. Can it be better sized to the window height/width so we get more space on a large window (less scrolling) and a smaller dialog on a small window? Ideally it should be draggable and resizeable.
3) I agree with Dan about the title. And the string should probably be "Add a new resource or activity" to match the launch button.
4) Agree that the launch link should look like a button!
5) The Next button should be on the right hand side and should be "Add". The Cancel button should be on the left hand side.

I have just pushed fixes to 1) and 3) now uses the same string.

Fred has just spotted that this is now failing QA Test MDLQA-1760

Hi All,

The functionality for this module is working great. However, I noticed that selecting the link from the last couple of sections cut the display of the dialog box. In most cases, selecting the last section, the dialog is displayed outside the viewing screen. *see attachment

This issue is also effecting IE browser.

Waiting other's feedback before decide it to fail/pass testing.

Testing with Firefox and screen size 1024x768m I found the same as Andrew that the box was too tall for the screen. Also if you open the dialog box then change your mind and close it, then click 'Add a resource or activity' in a different section, the dialog box opens right at the bottom of the screen, so with a small screen size you hardly see it (see attached screenshot).

Also just wondering whether activities could be listed before resources in the dialog box and the text changed to read 'Add an activity or resource'?

Sorry about this - it looks like notification.js is calling it's own centerDialogue after I've set centred the dialogue. Just working on a few now.

Great that the More help links open in a new window. However I'm finding the new window really small (see screenshot) even using a monitor with a huge screen size.

+1 for 'Add an activity or resource' (Sorry Dan!)

Discussed with Helen in PM - making popup default to 800x600

Dan's done points 1, and 3.

Point 2: Have fixed issue with dialogue centering - it was indeed an issue with M.Dialogue.centerDialogue being called after the chooserdialogue.center_dialogue() function. This was then masking an issue where the offset wasn't being set correctly.
The dialogue follows the following rules:

• default height of 540px with a fixed position
• if the browser height is smaller, it reduces the height to fit on the screen to prevent a user having to scroll the window as well as the dialogue
• if the browser height is below a minimum (300px) it stops resizing the dialogue and instead converts to absolute positioning and forces the user to scroll the window instead

4: Agreed it would be good as a button - can this be done in a later commit - I'd quite like to get the above fixes landed first
5: Cancel now on the left, Add now on the right

Will submit a pull branch in a moment

Branch: MDL-30617-master-patches
Log: https://git.luns.net.uk/moodle.git/shortlog/refs/heads/MDL-30617-master-patches
Diff: https://git.luns.net.uk/moodle.git/commitdiff/af75421c546b47934f02595b99e51e57c4b02e14

Cherry-pick: git fetch git://git.luns.net.uk/moodle.git MDL-30617-master-patches && git cherry-pick FETCH_HEAD
Checkout: git fetch git://git.luns.net.uk/moodle.git MDL-30617-master-patches && git checkout FETCH_HEAD

EDIT: Updated diff for and force pushed with text change as requested by MD

Pulled that.

Just realised that the dialogue title wasn't centred yet - sorry thought it had been done overnight. Additional commit:

Branch: MDL-30617-master-patches
Log: https://git.luns.net.uk/moodle.git/shortlog/refs/heads/MDL-30617-master-patches
Diff: https://git.luns.net.uk/moodle.git/commitdiff/2d1151817f11df973d537174ddb47addf7824f6a

pulled.

Our theme designer Dan has had a go at styling the link as a button.

Branch: MDL-30617-master-patches
Log: https://git.luns.net.uk/moodle.git/shortlog/refs/heads/MDL-30617-master-patches
Diff: https://git.luns.net.uk/moodle.git/commitdiff/462bd6367e4295d4945d417a8f4a4b282dfd5c33

EDIT: Sorry - updated the diff URL - wrong hash

Here's a screenshot of that button in Chrome on Linux

Dan, Re: Fred has just spotted that this is now failing QA Test MDLQA-1760

I've just tried to replicate without success. Seems to work exactly as expected for me.

Little annoyance: If any of the activities help or description panes have a scroll bar (for sure will once book is added). Then scrolling within the pane (mouse wheel) also causes the "dimmed" page to scroll once the pane is done. IMO it would be perfect to avoid scrolling of the "dimmed" page while the dialog is shown (make it more "modal").

Created MDL-33128 for listing activities before resources in the module chooser.

Passing this as I think that is the status and we want to send to moodle.git

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

Thanks!

Andrew, thanks again for the cool activity chooser in Moodle 2.3, which is now mentioned in the user docs here: http://docs.moodle.org/23/en/Course_homepage#Adding_activities_and_resources Thus I've removed the docs_required label for this issue.