You are here: Foswiki>Tasks Web>Item11917 (02 Dec 2012, GeorgeClark)Edit Attach

Item11917: Update JQueryPlugin simplemodal plugin to version 1.4.2

pencil
Priority: Enhancement
Current State: Closed
Released In: 1.1.6
Target Release: patch
Applies To: Extension
Component: JQueryPlugin
Branches: Release01x01 trunk
Reported By: ArthurClemens
Waiting For:
Last Change By: GeorgeClark
I know it has been deprecated, but it is still in use.

Upload to Extensions has failed, I've given up after 5 times server error.

-- ArthurClemens - 31 May 2012

Newer releases of simplemodal have been buggy with regards to resizing the modal dialog. That's why I did not update it for a log time. Besides, yes, better use ui::dialog. It is superior in so many ways.

This code is now out at JQueryPlugin. Could we please please please talk things like these thru before rushing out a new release!

After some initial testing, the new simplemodal behaves strange now while determining the height of the dialog window. Dialogs in NatEditPlugin, TopicInteractionPlugin and MetaCommentPlugin have a wrong height now all: showing whitespace underneath the dialog, or shrinking the dialog for no good reason. Checking the changes ...

Reopening and switched from "Enhancement" to "Urgent" frown, sad smile

This patch cures it:

--- jquery.simplemodal.uncompressed.js  (revision 14942)
+++ jquery.simplemodal.uncompressed.js  (working copy)
@@ -582,7 +582,7 @@
                                cw = s.o.autoResize && cw > mw ? mw : cw < mow ? mow : cw;
                        }
 
-                       s.d.container.css({height: ch, width: cw});
+                       s.d.container.css({width: cw});
                        s.d.wrap.css({overflow: (dh > ch || dw > cw) ? 'auto' : 'visible'});
                        s.o.autoPosition && s.setPosition();
                },

The plugin basically fails to compute the container dimensions correctly. In fact, I am not sure why it should alter the height at all. So I disabled this part all together.

I checked in this patch to trunk. Release branch pending.

Arthur could you please check?

-- MichaelDaum - 01 Jun 2012

With the patch I have problems with large modal windows.

If I use maxHeight: '90%', it works fine with the simplemodal code, not with this patch. The result is that the modal window is higher than the browser window, and the modal window extends beneath the browser top, obscuring the close button.

Possible reasons why height calculation may not work:
  • To use a fixed height, you must set both minHeight and maxHeight.
  • To set a size relative to the browser window, use percentages like maxWidth: '90%', maxHeight: '90%'.

There are also a couple of issues with height mentioned on the github issues page https://github.com/ericmmartin/simplemodal/pull/4.

I can't find an issue with the smaller dialogs on TopicInteractionPlugin (the large dialog won't appear, it leads to an attach page). So before looking into it further I would need to know what problems you see.

-- ArthurClemens - 16 Jun 2012

I already told you what problem there is: it does not compute the height of the simple modal container correctly.

To display lengthy modal dialogs it is best to create an overflow area yourself within a fixed size modal and let it display a scrollbar itself based on a max-height setting.

The other "solution" is to simply forget about simplemodal and switch to ui::dialog as I told you already.

-- MichaelDaum - 16 Jun 2012

"Arthur could you please check?" => your patch doesn't work for my cases. I try to understand what you mean by "fails to compute the container dimensions correctly". Unless you provide me with more specific information why it fails in your cases (and why your patch does work in those cases), my checking is done here.

My motivation to improve on this plugin is that I am sure it is being used. I have encountered it in the TagMe ajax interface. So a solution could be to fix those instances to use ui:dialog. However that will take more time.

-- ArthurClemens - 17 Jun 2012

Errors with your updates:

JQuerySimpleModalSnap1.pngJQuerySimpleModalSnap3.png

Fixes after my patch:

JQuerySimpleModalSnap2.pngJQuerySimpleModalSnap4.png

These are only two examples. There are more with even stranger heights being computed. So it is better to disable that feature by now. I already talked to the upstream maintainer over a year ago about this problem...

-- MichaelDaum - 09 Jul 2012
Topic revision: r25 - 02 Dec 2012, GeorgeClark
The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. See Copyright Statement. Creative Commons License    Legal Imprint    Privacy Policy