Skip to content

Use resizeobserver when available #213

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Mar 1, 2018
Merged

Use resizeobserver when available #213

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e19168f
Add size data to callback
movedoa Feb 6, 2018
2394495
Add size data to callback
movedoa Feb 6, 2018
cbcfa9a
Use ResizeObserver when available
movedoa Feb 6, 2018
82332b3
Fix IE11
movedoa Feb 14, 2018
4d8f3ee
Whoops, wrong indentation
movedoa Feb 14, 2018
d4f6590
Use Spaces
movedoa Mar 1, 2018
3a6a8f9
Cleanup
movedoa Mar 1, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 75 additions & 31 deletions src/ResizeSensor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@
* @constructor
*/
var ResizeSensor = function(element, callback) {

var observer;

/**
*
* @constructor
Expand All @@ -94,9 +97,9 @@
};

var i, j;
this.call = function() {
this.call = function(sizeInfo) {
for (i = 0, j = q.length; i < j; i++) {
q[i].call();
q[i].call(this, sizeInfo);
}
};

Expand Down Expand Up @@ -152,32 +155,50 @@
var expand = element.resizeSensor.childNodes[0];
var expandChild = expand.childNodes[0];
var shrink = element.resizeSensor.childNodes[1];
var dirty, rafId, newWidth, newHeight;

var dirty, rafId;
var size = getElementSize(element);
var lastWidth = size.width;
var lastHeight = size.height;

var reset = function() {
//set display to block, necessary otherwise hidden elements won't ever work
var invisible = element.offsetWidth === 0 && element.offsetHeight === 0;

if (invisible) {
var saveDisplay = element.style.display;
element.style.display = 'block';
}

var initialHiddenCheck = true, resetRAF_id;


var resetExpandShrink = function () {
expandChild.style.width = '100000px';
expandChild.style.height = '100000px';

expand.scrollLeft = 100000;
expand.scrollTop = 100000;

shrink.scrollLeft = 100000;
shrink.scrollTop = 100000;
};

if (invisible) {
element.style.display = saveDisplay;
var reset = function() {
// Check if element is hidden
if (initialHiddenCheck) {
if (!expand.scrollTop && !expand.scrollLeft) {

// reset
resetExpandShrink();

// Check in next frame
if (!resetRAF_id){
resetRAF_id = requestAnimationFrame(function(){
resetRAF_id = 0;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that variable name is not so great, but I'll adjust it in master directly after merge.

Copy link
Owner

@marcj marcj Mar 1, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also would like to throttle the calls, so maybe only every 100th requestAnimationFrame call do something, because I guess reading scrollTop and scrollLeft can be expensive. will adjust afterwards

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, didn't think about that

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marcj you might want to consider that throttling to that level (1 every 100 animationFrame) will be to slow to detect the resizing.
1 frame is around 1/60th of a second, so that makes it's around 1 check every 1,6 sec.

Use cases for this Initial detection are mostly for cases where something takes time to load, so it's hidden for 1 or 2 seconds at most.
But it's important to correctly detect when it's visible. (That is, at least, my own use case, where it's important to not miss the timing).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 check every 10th frame or so would be a good middle ground
every frame is to much 1/100 is not enough

Copy link
Owner

@marcj marcj Mar 1, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not always. When I use stuff in Angular for example, I have plenty of objects in the DOM but not visible (display: none). If there's a resize-sensor attached it will check for each element for its scrollTop/scrollLeft option every 1/100 second, so if I have some elements like that then the CPU will be molten, I guess.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant by since scrollTop/scrollTop === 0 is valid use-case, right, that when I have a initial hidden element lying at 0,0 (position: absolute; left: 0; top: 0), then scrollTop will always return 0, right? So it will never be detected as "it's now visible". Is my assumption wrong?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, HTML resize notification, while not native, should not be used extensively, and with great care.

In my case, I prefer exact performance instead of delayed detection. Maybe you can manage that with a configuration object ?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah if we don't find a good balanced value, we should go with a configuration option.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

about 2nd comment:
I think not, because the scrollTop will detect scrolling area positioning, not the element absolute positioning.
And this very property is being set to 10000 by the reset() function.

Here I'm exploiting a "bug" in how the browser manage the scroll properties: when an element is not visible, you can't set its scrollTop propertie: it will be immediately reset to 0.

So taking advantage of this and as it's on "our" detection HTML element, this will correctly detect when the element will become visible, without much cpu cost.


reset();
});
}

return;
} else {
// Stop checking
initialHiddenCheck = false;
}
}

resetExpandShrink();
};
element.resizeSensor.resetSensor = reset;

Expand All @@ -186,19 +207,17 @@

if (!dirty) return;

lastWidth = newWidth;
lastHeight = newHeight;
lastWidth = size.width;
lastHeight = size.height;

if (element.resizedAttached) {
element.resizedAttached.call();
element.resizedAttached.call(size);
}
};

var onScroll = function() {
var size = getElementSize(element);
var newWidth = size.width;
var newHeight = size.height;
dirty = newWidth != lastWidth || newHeight != lastHeight;
size = getElementSize(element);
dirty = size.width !== lastWidth || size.height !== lastHeight;

if (dirty && !rafId) {
rafId = requestAnimationFrame(onResized);
Expand All @@ -218,16 +237,41 @@
addEvent(expand, 'scroll', onScroll);
addEvent(shrink, 'scroll', onScroll);

// Fix for custom Elements
requestAnimationFrame(reset);
// Fix for custom Elements
requestAnimationFrame(reset);
}

if (typeof ResizeObserver !== "undefined") {
observer = new ResizeObserver(function(element){
forEachElement(element, function (elem) {
callback.call(
this,
{
width: elem.contentRect.width,
height: elem.contentRect.height
}
);
Copy link
Owner

@marcj marcj Mar 1, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about callback.call(this, elem.contentRect)?
Don't know the definition of contentRect tho.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I construct my own object because contentRect has more properties (like top,left,...)
So to avoid errors and keep it in line with the current implementation, I just pass these two

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright!

});
});
if (element !== undefined) {
forEachElement(element, function(elem){
observer.observe(elem);
});
}
}
else {
forEachElement(element, function(elem){
attachResizeEvent(elem, callback);
});
}

forEachElement(element, function(elem){
attachResizeEvent(elem, callback);
});

this.detach = function(ev) {
ResizeSensor.detach(element, ev);
if (typeof ResizeObserver != "undefined") {
observer.unobserve(element);
}
else {
ResizeSensor.detach(element, ev);
}
};

this.reset = function() {
Expand Down