Opened 10 years ago

Closed 10 years ago

#9394 closed defect (fixed)

dojox.fx.split has positioning issues

Reported by: cb1kenobi Owned by: nic
Priority: high Milestone: 1.4
Component: General Version: 1.3.1
Keywords: Cc:
Blocked By: Blocking:

Description

dojox.fx.split gets the dojo.coords() of the node to be split, but doesn't account for scenarios where a node between the body and the node to be split has a position of relative or absolute. It causes the effects position to be offset and in cases where the effect clipping=true, the effect may be mostly obscured.

The solution is to set the position of the parent node to relative and set the top and left styles of the node to the coords top and left instead of x and y.

This also means that a dojo.coords() can be replaced by dojo.getMarginBox().

Attachments (5)

dojox.fx.split.patch (2.9 KB) - added by cb1kenobi 10 years ago.
test_split_bug.zip (13.1 KB) - added by cb1kenobi 10 years ago.
dojox.fx.split.2.patch (2.6 KB) - added by cb1kenobi 10 years ago.
split.js.diff (2.6 KB) - added by nic 10 years ago.
test_split_bug2.html (2.7 KB) - added by nic 10 years ago.
another test case

Download all attachments as: .zip

Change History (15)

Changed 10 years ago by cb1kenobi

Attachment: dojox.fx.split.patch added

comment:1 Changed 10 years ago by cb1kenobi

I should note that the patch also does a bit of code cleanup. I also fixed a font size issue in the actual test file where nested <h1> tags caused the text to get huge.

Hmm, I should have probably also removed the quotes where ever padding/margin is set to zero. Could have saved a couple precious bytes. Oh well.

Changed 10 years ago by cb1kenobi

Attachment: test_split_bug.zip added

comment:2 Changed 10 years ago by cb1kenobi

Regarding the test case (zip file), put the files in the directory containing dojo and dojox.

comment:3 Changed 10 years ago by cb1kenobi

We can't apply this patch yet due to issue #9404. Once that is fixed, then revisit this bug and the patch.

Changed 10 years ago by cb1kenobi

Attachment: dojox.fx.split.2.patch added

comment:4 Changed 10 years ago by cb1kenobi

Summary: [patch] [cla] dojox.fx.split has positioning issuesdojox.fx.split has positioning issues

I changed back the code to use dojo.coords() and now things work great in FF, Chrome, and Opera for my test cases. IE however does not work. I hope the v2 patch will help as a starting point to solving IE compatibility.

comment:5 Changed 10 years ago by nic

I have a doubt about the parentNode position: when we set dojo.style(parentNode, "position", "relative") we are implicitly assuming that the "target" node has position != "absolute" and parentNode has no descendants with position:absolute; otherwise we are going to break the layout.
Am I right?
Also we have the same problem on dojox/fx/flip.js (where the node - absolutely positioned - is appended to dojo.body())

Changed 10 years ago by nic

Attachment: split.js.diff added

comment:6 Changed 10 years ago by nic

Patch based on the cb1kenobi's one.
This patch uses dojo.coords to place the effect node; also it considers the coords of the ancestor node having position absolute or relative, if present.
Tested on FF3, Safari 3.2.1 / win, IE7, Chrome 2.
I'm going to write a more complex test page.

comment:7 Changed 10 years ago by cb1kenobi

I applied Nicola's latest patch and it works beautifully in my 3 test cases in IE7 and FF3 (Mac and Win). As far as I'm concerned, this can be committed and this bug marked as fixed.

Changed 10 years ago by nic

Attachment: test_split_bug2.html added

another test case

comment:8 Changed 10 years ago by nic

Added another test page; now the effect node doesn't have positioning issues, but if the target node has margin left/top, the split position is wrong (eg not centered if there are 2 columns and 2 rows)

comment:9 Changed 10 years ago by dante

Owner: changed from dante to nic

@nic - take it away. keep in mind, in dojo 1.4 there is a new dojo.position which has proven to be more accurate with less adjusting needed for edge cases. perhaps it can help?

comment:10 Changed 10 years ago by nic

Resolution: fixed
Status: newclosed

(In [20258]) fixes #9394 and the margin issue too (fixed by the new dojo.coords implementation and dojo.position)

Note: See TracTickets for help on using tickets.