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.

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

overlay not in visible area, not possible to close #161

Closed
snozwoz opened this Issue Sep 6, 2010 · 17 comments

Comments

Projects
None yet
2 participants
@snozwoz

snozwoz commented Sep 6, 2010

whenthe overlay (with apple effect) is being shown, before onLoad has completed and during the animation, scroll down the screen with your mouse scroll wheel.
The result is that the overlay appears above your display area and consequently the user is not able to see the overlay, and close the overlay. so the user just sees the mask. the user is unable to do anything and has to click refresh.

@tipiirai

This comment has been minimized.

Show comment
Hide comment
@tipiirai

tipiirai Sep 11, 2010

Contributor

I could not find a (good) solution to this. Right now you can only avoid this issue by setting the configuration variable

fixed: false

Contributor

tipiirai commented Sep 11, 2010

I could not find a (good) solution to this. Right now you can only avoid this issue by setting the configuration variable

fixed: false

@snozwoz

This comment has been minimized.

Show comment
Hide comment
@snozwoz

snozwoz Sep 12, 2010

i am guessing but the effect code gets it target position before the animation starts then moves the object. The code needs changing so the target position is continuously updated during the animation.
The temporary fix i have implemented is to check the position after the overlay is fully open and animation is complete and check it is visible. it it is not i move it into view without any animation. The solution is crude and fixes the issue but not in a nice way.

snozwoz commented Sep 12, 2010

i am guessing but the effect code gets it target position before the animation starts then moves the object. The code needs changing so the target position is continuously updated during the animation.
The temporary fix i have implemented is to check the position after the overlay is fully open and animation is complete and check it is visible. it it is not i move it into view without any animation. The solution is crude and fixes the issue but not in a nice way.

@tipiirai

This comment has been minimized.

Show comment
Hide comment
@tipiirai

tipiirai Sep 13, 2010

Contributor

can I see your fix?

thanks.

Contributor

tipiirai commented Sep 13, 2010

can I see your fix?

thanks.

@snozwoz

This comment has been minimized.

Show comment
Hide comment
@snozwoz

snozwoz Sep 16, 2010

my fix is not nice:
onLoad calls a function with:

var top = content.offset().top - $(window).scrollTop();
if(top<0){
overlayDiv.css("top","0px");
overlayDiv.data("img").css("top","0px");
}

Also, becuase the popup is fixed i dont want it any larger than the users screen size since they will not be able to scroll over the hidden content.

var maxHeight = $(window).height() - top;
content.css("max-height",maxHeight + "px");

you could do the same with width. The issue is with position fixed content is the user can not scroll to it or around it. So i therefore have to check it is visible with both scroll position and with and height.

The other issue is that you set isOpened() to true before the apple animation starts not after. So any calls to this function returns true even though it not actually open yet.

A good example of this issue is:

  1. on document.ready you open a overlay popup
  2. the URL is widget.html#some_location_down_the_page

the overlay starts opening before the browser scrolls down the page to #some_location_down_the_page. The result is the overlay when open is out of sight up the page somewhere. because it is position fixed you cant scroll back to it either. The user just sees the mask and no close button.

Another example is as i mentioned before to open the overlay and use your mouse wheel to scroll during the animation.

I think you could fix this quite easily. The issue is the animated content is positioned relative to the document. The document can move if the scrolled. The solution is to put the animated content inside a static container.
So simply wrap the overlay with a div whose css position:fixed; top:0; left:0, and also set the width and height of it to match the browser window size, and finally overflow:auto.
You then dont need to make the overlay fixed or worry about it being visible. if the overlay is larger than the browser the container will show scroll bars. If the browser scrolls during the animation the container will remain static so the overlay will remain in perfect position.

snozwoz commented Sep 16, 2010

my fix is not nice:
onLoad calls a function with:

var top = content.offset().top - $(window).scrollTop();
if(top<0){
overlayDiv.css("top","0px");
overlayDiv.data("img").css("top","0px");
}

Also, becuase the popup is fixed i dont want it any larger than the users screen size since they will not be able to scroll over the hidden content.

var maxHeight = $(window).height() - top;
content.css("max-height",maxHeight + "px");

you could do the same with width. The issue is with position fixed content is the user can not scroll to it or around it. So i therefore have to check it is visible with both scroll position and with and height.

The other issue is that you set isOpened() to true before the apple animation starts not after. So any calls to this function returns true even though it not actually open yet.

A good example of this issue is:

  1. on document.ready you open a overlay popup
  2. the URL is widget.html#some_location_down_the_page

the overlay starts opening before the browser scrolls down the page to #some_location_down_the_page. The result is the overlay when open is out of sight up the page somewhere. because it is position fixed you cant scroll back to it either. The user just sees the mask and no close button.

Another example is as i mentioned before to open the overlay and use your mouse wheel to scroll during the animation.

I think you could fix this quite easily. The issue is the animated content is positioned relative to the document. The document can move if the scrolled. The solution is to put the animated content inside a static container.
So simply wrap the overlay with a div whose css position:fixed; top:0; left:0, and also set the width and height of it to match the browser window size, and finally overflow:auto.
You then dont need to make the overlay fixed or worry about it being visible. if the overlay is larger than the browser the container will show scroll bars. If the browser scrolls during the animation the container will remain static so the overlay will remain in perfect position.

@snozwoz

This comment has been minimized.

Show comment
Hide comment
@snozwoz

snozwoz Sep 16, 2010

Further to my response here, i tried my solution as a test. it works.
it needs implementing within your overlay code however as when you create the IMG layer it is outside my container. The result is that as i scroll up and down the page the overlay stays static but he background image does not. I HAVE ONLY TESTED THIS IN CHROME.
this is what i did.

<div class="overlayContainer">
<div id="overlay"><div id="contentWrap"></div></div>
</div>

CSS
.overlayContainer{position:fixed;top:0;left:0;}

$("#overlay").overlay({
fixed:false,
effect: 'apple'
});

To implement this nicely when the user selects fixed:true the code should wrap the overlay with this wrapper with position fixed, rather than apply position fixed to the overlay. when it creates the IMG layer with the background that too needs to be inside this container.

I hope this help
kind regards
austin
PS; Your library is great.

snozwoz commented Sep 16, 2010

Further to my response here, i tried my solution as a test. it works.
it needs implementing within your overlay code however as when you create the IMG layer it is outside my container. The result is that as i scroll up and down the page the overlay stays static but he background image does not. I HAVE ONLY TESTED THIS IN CHROME.
this is what i did.

<div class="overlayContainer">
<div id="overlay"><div id="contentWrap"></div></div>
</div>

CSS
.overlayContainer{position:fixed;top:0;left:0;}

$("#overlay").overlay({
fixed:false,
effect: 'apple'
});

To implement this nicely when the user selects fixed:true the code should wrap the overlay with this wrapper with position fixed, rather than apply position fixed to the overlay. when it creates the IMG layer with the background that too needs to be inside this container.

I hope this help
kind regards
austin
PS; Your library is great.

@tipiirai

This comment has been minimized.

Show comment
Hide comment
@tipiirai

tipiirai Sep 17, 2010

Contributor

this is now fixed. thanks to your latest comment. here is the commit:

http://github.com/jquerytools/jquerytools/commit/12f70d55b37ea91a19e61929e9fa47a9cd97c663

and here is the updated apple effect:

http://github.com/jquerytools/jquerytools/raw/12f70d55b37ea91a19e61929e9fa47a9cd97c663/src/overlay/overlay.apple.js

tested it with fixed and absolute positioning and everything works fine.

THANKS!

Contributor

tipiirai commented Sep 17, 2010

this is now fixed. thanks to your latest comment. here is the commit:

http://github.com/jquerytools/jquerytools/commit/12f70d55b37ea91a19e61929e9fa47a9cd97c663

and here is the updated apple effect:

http://github.com/jquerytools/jquerytools/raw/12f70d55b37ea91a19e61929e9fa47a9cd97c663/src/overlay/overlay.apple.js

tested it with fixed and absolute positioning and everything works fine.

THANKS!

@snozwoz

This comment has been minimized.

Show comment
Hide comment
@snozwoz

snozwoz Sep 17, 2010

no worries. your solution is different to mine, I dont see any container being added but I can see you have changed how you position the IMG, does this solve the issue?
What is the best way for me to test this and give you feedback. i have currently just used your download page to get all the projects. you dont have a nightly download page do you?
if i copy the code into a new js file and load this after will it overwrite the existing code? Or should i download the project less the apple effect, and then load this new code?

ta

snozwoz commented Sep 17, 2010

no worries. your solution is different to mine, I dont see any container being added but I can see you have changed how you position the IMG, does this solve the issue?
What is the best way for me to test this and give you feedback. i have currently just used your download page to get all the projects. you dont have a nightly download page do you?
if i copy the code into a new js file and load this after will it overwrite the existing code? Or should i download the project less the apple effect, and then load this new code?

ta

@tipiirai

This comment has been minimized.

Show comment
Hide comment
@snozwoz

This comment has been minimized.

Show comment
Hide comment
@snozwoz

snozwoz Sep 21, 2010

hi,
some feedback for you.

  1. you are very clever and know a lot about javascript.

  2. for most cases it is fixed. I could create one instance where it was still broken.

    a) If i click a link and scroll during the animation it is fixed.

    b) If i set document.ready() to open a overlay on document load and scroll whilst the page is loaded then it is broken.It is broken in a different way though. If i am scrolling down then it appears further down the page than it should. (it use to appear up the page in the position before scrolling). So now it looks like the animation is fine but the target position is wrong.
    I have tested in both chrome and IE 7 / 8 with the same results.

snozwoz commented Sep 21, 2010

hi,
some feedback for you.

  1. you are very clever and know a lot about javascript.

  2. for most cases it is fixed. I could create one instance where it was still broken.

    a) If i click a link and scroll during the animation it is fixed.

    b) If i set document.ready() to open a overlay on document load and scroll whilst the page is loaded then it is broken.It is broken in a different way though. If i am scrolling down then it appears further down the page than it should. (it use to appear up the page in the position before scrolling). So now it looks like the animation is fine but the target position is wrong.
    I have tested in both chrome and IE 7 / 8 with the same results.

@snozwoz

This comment has been minimized.

Show comment
Hide comment
@snozwoz

snozwoz Sep 21, 2010

my fix for this is onLoad to check the position with this:

var top = content.offset().top - $(window).scrollTop();
var sh= $(window).height();
if(top<20 || top>(sh/2)){
overlayDiv.css("top","20px");
overlayDiv.data("img").css("top","20px");
}

If you are unable to find the cause then maybe you shoudl put this final check in your code

snozwoz commented Sep 21, 2010

my fix for this is onLoad to check the position with this:

var top = content.offset().top - $(window).scrollTop();
var sh= $(window).height();
if(top<20 || top>(sh/2)){
overlayDiv.css("top","20px");
overlayDiv.data("img").css("top","20px");
}

If you are unable to find the cause then maybe you shoudl put this final check in your code

@snozwoz

This comment has been minimized.

Show comment
Hide comment
@snozwoz

snozwoz Sep 21, 2010

forget what i have said here, i found the root cause:
scroll down the page a bit, now click a button and the overlay is opened out of sight.
After playing with the code i see the cause is the following lines, which with the following change fixes the issue.

var itop = conf.start.top || Math.round(w.height() / 2),
ileft = conf.start.left || Math.round(w.width() / 2);

if (trigger) {
var p = getPosition(trigger);
itop = p.top;
ileft = p.left;
}

// I HAVE ADDED THE FOLLOWING CHECK HERE 1 OF 2 CHANGES
if(conf.fixed){
itop -= w.scrollTop();
ileft -= w.scrollLeft();
}
// initialize background image and make it visible
img.css({
position: position,
top: itop,
left: ileft,
width: 0,
zIndex: conf.zIndex
}).show();

// I HAVE ADDED THE FOLLOWING CHECK HERE 2 OF 2 CHANGES
if(!conf.fixed){
pos.top += w.scrollTop();
pos.left += w.scrollLeft();
}
pos.position = position;
overlay.css(pos);

This might need some more testing

As you no doubt already know with fixed positioning the co-ordinates are relative to the browser and not the document. so you need to use scrollTop to calculate the start position relative to the browser, but you dont want it for the end position.

hope this helps.

snozwoz commented Sep 21, 2010

forget what i have said here, i found the root cause:
scroll down the page a bit, now click a button and the overlay is opened out of sight.
After playing with the code i see the cause is the following lines, which with the following change fixes the issue.

var itop = conf.start.top || Math.round(w.height() / 2),
ileft = conf.start.left || Math.round(w.width() / 2);

if (trigger) {
var p = getPosition(trigger);
itop = p.top;
ileft = p.left;
}

// I HAVE ADDED THE FOLLOWING CHECK HERE 1 OF 2 CHANGES
if(conf.fixed){
itop -= w.scrollTop();
ileft -= w.scrollLeft();
}
// initialize background image and make it visible
img.css({
position: position,
top: itop,
left: ileft,
width: 0,
zIndex: conf.zIndex
}).show();

// I HAVE ADDED THE FOLLOWING CHECK HERE 2 OF 2 CHANGES
if(!conf.fixed){
pos.top += w.scrollTop();
pos.left += w.scrollLeft();
}
pos.position = position;
overlay.css(pos);

This might need some more testing

As you no doubt already know with fixed positioning the co-ordinates are relative to the browser and not the document. so you need to use scrollTop to calculate the start position relative to the browser, but you dont want it for the end position.

hope this helps.

@snozwoz

This comment has been minimized.

Show comment
Hide comment
@snozwoz

snozwoz Sep 21, 2010

it works very nicely thank you. tested in chrome and IE 7.
when are you going to make this live?

cheers
Austin

snozwoz commented Sep 21, 2010

it works very nicely thank you. tested in chrome and IE 7.
when are you going to make this live?

cheers
Austin

@tipiirai

This comment has been minimized.

Show comment
Hide comment
@tipiirai

tipiirai Sep 21, 2010

Contributor

this/next week

Contributor

tipiirai commented Sep 21, 2010

this/next week

@snozwoz

This comment has been minimized.

Show comment
Hide comment
@snozwoz

snozwoz Sep 21, 2010

The only other improvement i might suggest; is to not allow the overlay width and height to be any larger than the screen visible area will permit when position is fixed?
Since due to it being fixed the user will not be able to scroll around it.

my onLoad function does this in a crude way with:

var top = content.offset().top - $(window).scrollTop();
var sh= $(window).height();
var maxHeight = (sh-top-30-10)+"px";
content.css("max-height",maxHeight);

I guess you could provide a configuration parameter to allow this restriction to be switchable.

snozwoz commented Sep 21, 2010

The only other improvement i might suggest; is to not allow the overlay width and height to be any larger than the screen visible area will permit when position is fixed?
Since due to it being fixed the user will not be able to scroll around it.

my onLoad function does this in a crude way with:

var top = content.offset().top - $(window).scrollTop();
var sh= $(window).height();
var maxHeight = (sh-top-30-10)+"px";
content.css("max-height",maxHeight);

I guess you could provide a configuration parameter to allow this restriction to be switchable.

@tipiirai

This comment has been minimized.

Show comment
Hide comment
@tipiirai

tipiirai Sep 21, 2010

Contributor

the maximum width is set with CSS

Contributor

tipiirai commented Sep 21, 2010

the maximum width is set with CSS

@snozwoz

This comment has been minimized.

Show comment
Hide comment
@snozwoz

snozwoz Sep 21, 2010

yes, i would normally agree except the max width or hight needs to be equal to the visible screen size. cant do this with css alone ?

snozwoz commented Sep 21, 2010

yes, i would normally agree except the max width or hight needs to be equal to the visible screen size. cant do this with css alone ?

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment