Moodle

With this function it's possible now to hide the homelink in a popup window. A big usability improvement because people can't move to home and use this popup window as main window and get confused.

<?php if (!is_in_popup()) { echo $homelink; } ?>

changed "get_in_" to new function name "is_in_...".

Sounds good to me. We just need to add inpopup=1 as a parameter to all popups (and pages within popups, like the filemanager).

Some places to add the parameter:

• glossary links.
• messaging popups.
• file manager.

After chat with Urs, this could be the plan:

1) add the parameter everywhere we need it.
2) edit themes to suppress header and footer printing if inpopup
3) drop old css (used to hide headers manually)
4) Hack the body tag to add one more class "inpopup", It can be used to change margins and other things.

Ciao

Looking at the code, there seems to be a lot of places where popups are created without using the button_to_popup_window and link_to_popup_window functions. This could be an occasion to fix this.. after which, adding a parameter every time a popup is created will be easy.

Then, looking at glossary, I don't quite understand why it sometimes ends up being a popup and sometimes not. Have a look at http://moodle.org/mod/forum/discuss.php?d=75760&parent=377450 : Applet and Java open in the same window, while applet, firefox, IE and java open in a popup.

Yep, to use central functions in as many places as possible is always a good idea. +1 for that.

About the glossary... it seems to be another bug! Looking to it right now. Will keep you informed, but it's a bug, for sure.

Ciao

Hi again,

after looking your example above... I really think it isn't a bug at all. Those "Applet" and "Java" not being displayed in a popup are there because the links are in the posted (by hand) message and aren't calculated automatically by the glossary filter at all.

So I think we can ignore that, glossary filter ALWAYS opens entries in popups..

Ciao

Ok, that's good.

It won't be easy to get everything to use the _to_popup_window functions, especially not the glossary filter. I think I'll just add something to the link in there.

There is also a popup in the TeX filter, but there is no themeing done there : it just displays the formula.

I'll commit the modified _to_popup_window functions later today. Everything else will come in slowly, as I learn my way in the code.

After discussing with Martin, I added two css classes to windows : "nonavigation" and "noheading". A popup window usually combines both, but it's now possible to distinguish easily between all possible cases.

Why did you introduce those CLASSes? I don't see the advantage of the two "negative" no-CLASSes. Why not simply use class="inpopup"? "Inpopup" is needed to change page content display via CSS.

Skipping the header/navigation is better done in the header via PHP with the function is_in_popup() like if (is_in_popup()) { ... }. Then this parts of the page dont need to go through the wires to the browser.

Or you may add the "inpopup" CLASS every time the page opens in a popup window.

Yep,

agree with Urs,

IMO what we are going to do is to SKIP the whole header (and footer) output from themes header.php (and footer.php). I fact it's implemented in custom_corners right now. One simple "if inpopup" to control the print of the whole header (and footer) div.

So, at least in the case of popup windows, those "nonavigation" and "noheading" classes are useless because, by definition the whole header isn't going to be printed.

So I would:

1) Add the "inpopup" class if we are in a popup window. To be able to use common selectors for all the popup windows.
2) If you find any use for the "nonavigation" and "noheading" classes, apart from the "inpopup" one, no problem here to add them too when needed. But IMO they are 100% different from the "inpopup" idea.

Ciao

Sorry about the way-too-short length of my last message, this was clearly not enough explanations. I'll take what time is left before leaving for the weekend to try to fix this..

After re-reading the whole issue, I'm not sure of the exact use case we are trying to get to work.

First, I agree that classes shouldn't be negative.. will think about something better. These are the one that we came up with, after talking with Martin. The idea with these classes is that the layout changes are (I think) related to the way we want the content to be presented, not to the fact it's presented in a popup. In practice, where do we stop adding the "inpopup=1" to links? In a (poped-up) glossary entry, there's a link on the user name linking to it's profile, and in the profile, a link back to the homepage..

Then, if the user deactivated javascript, do we still want the layout to be "popup layout" even though the window might replace the previous one? The *_to_popup_window() functions are clearly labeled as "working even without javascript", there seems to be something historical about this.

I'm still learning my way in Moodle's code, this issue is an occasion to dig a bit deeper.. Help & explanations are much appreciated.

Hopefully, this will all be clear in my head on Monday morning!

Pushing this to after 1.9 release.

Following a conversation with Eloy, here's the first part of a proposed patch :

Index: lib/javascript.php
===================================================================
RCS file: /cvsroot/moodle/moodle/lib/javascript.php,v
retrieving revision 1.36.2.1
diff -u -F ^f -r1.36.2.1 javascript.php
— lib/javascript.php 3 Dec 2007 15:42:09 -0000 1.36.2.1
+++ lib/javascript.php 14 Mar 2008 06:39:27 -0000
@@ -33,6 +33,11 @@

function openpopup(url,name,options,fullscreen) {
fullurl = "<?php echo $CFG->httpswwwroot ?>" + url;
+ if (fullurl.indexOf('?') >= 0) { + fullurl += '&inpopup=1'; + } else { + fullurl += '?inpopup=1'; + }
windowobj = window.open(fullurl,name,options);
if (fullscreen) {
windowobj.moveTo(0,0);

At this time, this would work only in custom_corners (which is the only theme using is_in_popup).

This does not solve the problem of links available in the popup, which will lead to a page withouth the proper class. Any insight on this? I am contemplating using dhtml to do that, however hackish this is all becoming. Eloy, Urs, what did you have in mind for that?

Well, really, navigation within popups is a problem... mainly because we cannot assume if we are in a popup or no. There are two ways to address this, IMO:

1) Hack pages being susceptible of being displayed in popup, to read check the is_in_popup() function adding the required &inpopup=1 when necessary. We could start using core functions to generate all those links and those functions should be able to automatically add the inpopup=1 if specified in the call.

2) Use some js to add it automatically. But this will apply to all the links, and I think it's not a good idea.

So, one more detailed plan could be:

1) Ensure we have some "make_potentially_popup_link"or so function (note it's a fake name!) able to process links. It will have one optional parameter $autopopup=false, so, if set to true, then the function will check is_in_popup() and will add the inpopup=1 to the link.

2) The same should be existent for buttons, I guess (if we have buttons requiring navigation in popupmode).

3) Perhaps the element_to_popup_window() and family could be reused for this, not sure. perhaps to have one different function as commented in 1) is better.

4) Start applying the make_potentially_popup_link() function to all the links requiring navigation in popup-mode. That way the npopup=1 will arrive in the next request and everything will be properly detected.

Just a draft, let MD and others take a look over this.

Ciao

PS: The hack above about to pass the the inpopup=1 looks perfect.

By 'using core functions to generate links', do you mean having a new function like link($text, $path, $options) that we would use instead of "echo '<a href..." ?

I would be in favor of this, but this would be a reallly big change...

This is being fixed by the MDL-19077 work.