Opened 10 years ago

Closed 9 years ago

Last modified 9 years ago

#10640 closed defect (fixed)

[patch] smoothScroll scrolls too far within offset node

Reported by: fritteritter Owned by: nic
Priority: low Milestone: 1.5
Component: Dojox Version: 1.4.0
Keywords: scroll Cc: nic
Blocked By: Blocking:

Description

There is an incorrect calculation in determining the position of the target node in a smoothScroll animation when the node which is being scrolled is offset from the top of the client window. The current code assumes that the distance between the target node and the top of the window is the distance that the parent node should be scrolled, so when the parent node is offset this results in scrolling too far.

The following example shows that, while each pane should scroll to the number 3 and stop, the panes which are farther from the top of the window scroll farther past the 3.

<html>
<head>
<script src="dojo.js" type="text/javascript"></script>
<style type="text/css">
.box {
	height: 50px;
	background: #000;
	color: #fff;
	margin: 1px 0;
	font-size: 3em;
	line-height: 50px;
	text-align: center;
}

.pane {
	position: absolute;
	top: 0px;
	left: 0px;
	width: 100px;
	height: 100px;
	overflow: auto;
	border: 3px solid #f00;
}

.target {
	background: #090;
}
</style>
</head>
<body>
<div class="pane" id="pane_1">
	<div class="box">1</div>
	<div class="box">2</div>
	<div class="box target" id="target_1">3</div>
	<div class="box">4</div>
	<div class="box">5</div>
	<div class="box">6</div>
	<div class="box">7</div>
	<div class="box">8</div>
	<div class="box">9</div>
	<div class="box">10</div>
</div>

<div class="pane" id="pane_2" style="top: 100px; left: 110px;">
	<div class="box">1</div>
	<div class="box">2</div>
	<div class="box target" id="target_2">3</div>
	<div class="box">4</div>
	<div class="box">5</div>
	<div class="box">6</div>
	<div class="box">7</div>
	<div class="box">8</div>
	<div class="box">9</div>
	<div class="box">10</div>
</div>


<div class="pane" id="pane_3" style="top: 200px; left: 220px;">
	<div class="box">1</div>
	<div class="box">2</div>
	<div class="box target" id="target_3">3</div>
	<div class="box">4</div>
	<div class="box">5</div>
	<div class="box">6</div>
	<div class="box">7</div>
	<div class="box">8</div>
	<div class="box">9</div>
	<div class="box">10</div>
</div>

<script type="text/javascript">
dojo.require("dojox.fx.scroll");
dojo.addOnLoad(function() {
	dojox.fx.smoothScroll({
		node: dojo.byId('target_1'),
		win: dojo.byId('pane_1')
	}).play();

	dojox.fx.smoothScroll({
		node: dojo.byId('target_2'),
		win: dojo.byId('pane_2')
	}).play();

	dojox.fx.smoothScroll({
		node: dojo.byId('target_3'),
		win: dojo.byId('pane_3')
	}).play();
});
</script>
</body>
</html>

The patch will fix this issue but has not been regression tested.

Attachments (3)

dojox.fx.scroll.patch (467 bytes) - added by fritteritter 10 years ago.
Subtracted absolute x and y offset of the parent from the target coords to correct the scrolling distance calculation.
scroll.js.patch (1.8 KB) - added by nic 10 years ago.
smoothscroll.html (2.9 KB) - added by Nick Fenwick 9 years ago.
My smoothscroll test page, uses google cdn

Download all attachments as: .zip

Change History (10)

Changed 10 years ago by fritteritter

Attachment: dojox.fx.scroll.patch added

Subtracted absolute x and y offset of the parent from the target coords to correct the scrolling distance calculation.

comment:1 Changed 10 years ago by Adam Peller

Cc: nic added
Owner: changed from Adam Peller to dante

Changed 10 years ago by nic

Attachment: scroll.js.patch added

comment:2 Changed 10 years ago by nic

Subtracted the x, y offset without changing the target argument; used dojo.position instead of dojo.coords

comment:3 Changed 9 years ago by dante

Owner: changed from dante to nic

@nic - wfm. patch is seemingly trivial enough to not require a CLA

comment:4 Changed 9 years ago by nic

Resolution: fixed
Status: newclosed

(In [21827]) smoothScroll positioning fix. fixes #10640

comment:5 Changed 9 years ago by bill

Milestone: tbd1.5

comment:6 Changed 9 years ago by Nick Fenwick

Hi guys.. I have 1.5.0b2 here, which looks like it contains the patch content, and seem to still have this problem.

I knocked up a test page of my own before trying the one in this bug description, both seem broken. I'll attach my test page for reference, or you may find it at http://neekfenwick.homeip.net:8080/test/smoothscroll.html if my server is running. I presume I'm suffering from the same problem that this bug describes, or a close variant of it.

Does this bug need reopening?

Changed 9 years ago by Nick Fenwick

Attachment: smoothscroll.html added

My smoothscroll test page, uses google cdn

comment:7 Changed 9 years ago by Nick Fenwick

OK, I'm an idiot, my test page using google cdn uses dojo 1.4 which of course doesn't contain the patch. Stick to http://neekfenwick.homeip.net:8080/test/smoothscroll.html. Anyway, nicrizzo has confirmed a problem.

Raised #11110 to deal with it. Damn. Missed 11111.

Note: See TracTickets for help on using tickets.