OpenLayers OpenLayers

Ticket #1469 (closed feature: fixed)

Opened 2 years ago

Last modified 2 years ago

Automatically resize popups if included content changes size

Reported by: crschmidt Assigned to: pagameba
Priority: major Milestone: 2.7 Release
Component: Popup Version: SVN
Keywords: Cc:
State: Complete

Description

When popups contain things like <img> tags which don't have width and height (or correct width/height), the autosizing content doesn't work right until the image has loaded -- but unfortunately, this is *after* the popup has determined its size. To account for this, automatically resize the popup after images load, if images loading changes the popup size.

It is likely that the 'right' solution to this is to explore the DOM tree and find images, registering events to them, and then using those events to trigger a resize.

Attachments

popup.patch (1.9 kB) - added by crschmidt on 08/03/08 08:00:32.
Popup.html (1.4 kB) - added by pagameba on 08/05/08 10:04:35.
autoresize test for popup
registerImageListeners.2.patch (4.4 kB) - added by euzuro on 08/28/08 16:44:37.
cr5 pointed out race condition where images load in the interim between updateSize() and the call to registerImageListeners(). I have switched the order around accordingly.
registerImageListeners.3.patch (4.5 kB) - added by euzuro on 08/28/08 16:59:53.
adding a test to make sure the popup is visible before we pan the whole map to accomodate it
registerImageListeners.patch (4.5 kB) - added by euzuro on 08/28/08 17:00:25.
adding a test to make sure the popup is visible before we pan the whole map to accomodate it

Change History

03/30/08 21:08:56 changed by euzuro

  • status changed from new to assigned.

07/28/08 11:52:54 changed by crschmidt

  • priority changed from minor to major.

08/03/08 08:00:32 changed by crschmidt

  • attachment popup.patch added.

08/03/08 08:11:48 changed by crschmidt

  • owner deleted.
  • status changed from assigned to new.
  • state set to Review.

08/05/08 09:12:32 changed by pagameba

  • owner set to pagameba.
  • status changed from new to assigned.

This patch looks really good, but I think there should be a test for this to catch future regressions. I'll work on adding one.

08/05/08 10:04:35 changed by pagameba

  • attachment Popup.html added.

autoresize test for popup

08/05/08 10:22:00 changed by pagameba

Ok, I failed at creating the test and I don't know what's going on. I started just trying to measure the size of the popup without opening a window and kept getting w=0,h=0 so I tried cobbling together the example from popups.html into a t.open_window thinking that perhaps measuring doesn't work if the window isn't visible. But it still doesn't work. I was planning to have two tests, one that ensures autoSize is working and one that loads a largish image so that the autoSize would have to adjust its size after the image loaded (to test Chris' patch). I'm keen to do the tests but I am blocked on this so any help would be appreciated.

08/28/08 16:40:05 changed by euzuro

ok, new patch added. all tests pass in ff2 and ie7.

regarding the tests, i find that this sort of thing is near impossible and incredibly frustrating to code.

...which is why I have chosen to replace it with a small mod to the general popup acceptance test, 'popupMatrix.html'

the other change of interest here is that I am moving the call to updateSize() in the setContentsHTML() function into the conditional. There is no reason to call updateSize() if we are not actually changing the contents of contentDiv. If people are using setContentHTML() (without changing 'contentHTML' or the contents of contentDiv) to resize the popup, that is wrong... they should just be using updateSize().

08/28/08 16:44:37 changed by euzuro

  • attachment registerImageListeners.2.patch added.

cr5 pointed out race condition where images load in the interim between updateSize() and the call to registerImageListeners(). I have switched the order around accordingly.

08/28/08 16:59:53 changed by euzuro

  • attachment registerImageListeners.3.patch added.

adding a test to make sure the popup is visible before we pan the whole map to accomodate it

08/28/08 17:00:25 changed by euzuro

  • attachment registerImageListeners.patch added.

adding a test to make sure the popup is visible before we pan the whole map to accomodate it

08/28/08 17:01:49 changed by crschmidt

  • state changed from Review to Commit.

08/28/08 17:05:46 changed by euzuro

  • status changed from assigned to closed.
  • state changed from Commit to Complete.
  • resolution set to fixed.

(In [7887]) Adding registerImageListener() function that sits around and waits until images load in our popups... and when they do, it calls updateSize() so that the popup is sized correctly. thanks for the sharp review cr5 (Closes #1469)