Skip to content
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

Modal type step top position is wrong #61

Open
Nogostradamus opened this issue Mar 27, 2015 · 9 comments
Open

Modal type step top position is wrong #61

Nogostradamus opened this issue Mar 27, 2015 · 9 comments

Comments

@Nogostradamus
Copy link

When page is long and your scroll position is on the bottom the modal type step is displayed on the very top on the screen (top 25%):
$jpwTooltip.css({
'position': 'absolute',
'left': '50%',
'top': '25%',
'margin-left': -(parseInt(step.popup.width, 10) + 60) / 2 + 'px',
'z-index': '999999'
});
To see that you need to scroll up.
I think it should calculate the scroll position like that:
'top': 'calc('+$(document).scrollTop()+'px + 25%)'
That way it will be displayed in top25% of the current position. The scroll is not triggered in most of the cases because of the complex conditions for scroll. This will prevent that and you won't seen the blank screen with hidden content on top.
In my project I have removed scroll for modals and display that on the current scroll and removed all conditions for scroll to be active all the time. So I can see nice transitions fade in/out for each step.

@jwarby
Copy link
Owner

jwarby commented Mar 29, 2015

Hi, thanks for raising this. Under what conditions are you seeing this behaviour occur? The plugin should be scrolling whichever element needs to be scrolled back to 0. You mentioned complex conditions for the scroll?

@Nogostradamus
Copy link
Author

line 268: conditions for scrolling
I have console logged 3 things: scrollTop, scrollTo and maxScroll
In most cases in my app this condition is not triggered because of this conditions. I have application with many panels and each panel have 'help' icon. If panel is on the bottom of the page and I want to highlight something on top of the page, scroll stays on bottom. The same story with modal type. Modal type is displayed on top but page stays on bottom. And with modal I don;t want to scroll to the top to see modal, I want to stay with current view, that's why I have changed that.
Hope that helps

jwarby pushed a commit that referenced this issue Mar 30, 2015
…toScroll:

false` will now stop the plugin from scrolling - #61
@jwarby
Copy link
Owner

jwarby commented Mar 30, 2015

Hi @Nogostradamus, thanks for your reply. There are actually two additional bugs here:

  1. the element which should be scrolled to 0 for modal steps is not being scrolled
    • the default scroll-to-0 behaviour can't be overridden anyway, which you would need to do in your case (if scrolling did work). It can't be overridden because of issue 2 below.
  2. setting the autoScroll option to false doesn't disable the scrolling. The option is actually broken and doesn't do anything at all.

I've merged your code into a new dev branch. I've put an additional commit on top, which fixes the 2nd issue mentioned above.

Would you mind checking that the dev branch version works for you? In theory, it shouldn't make any difference in your case due to the scrolling bug.

@Nogostradamus
Copy link
Author

Hi,
In the dev version modal type position is fixed allright. It shows where the current page scroll is.
But none on the tooltip steps are scrolled up or down. I have to scroll manually to see the highlight content. For up, animate scroll is not triggered at all. Condition if is not meet.
(trace booleans:

  1. true
    a. false true
    b. true false
    c. false)
    It's triggered to scroll back to the bottom but it's not working.
    (trace:
    1.true
    a. false true
    b. false false
    c. true)
    No effect at all. scrollTo var is correct number though. I will try to find a solution and debugg it here when have more time. Hope that helps for now.

Also I have one more issue but I will start a new topic with that.

@Nogostradamus
Copy link
Author

I have played with that for a while and I found out that there is an issue with scrollTarget. scrollTarget is body element. When using $('html, body') instead is fine. It applies to both modal and tooltips. Also that's why conditional 1.a.b.c is not triggered as well.

jwarby pushed a commit that referenced this issue Apr 1, 2015
…'html' to

the fallback 'body' selector - related #61
@jwarby
Copy link
Owner

jwarby commented Apr 1, 2015

Thanks for taking the time to look into this @Nogostradamus! I've added two commits to the dev branch, one for the 'html, body' selector, and one for issue #63 about the hole placement. Would you mind reviewing and seeing if everything is now fixed?

@Nogostradamus
Copy link
Author

It's better now. Scrolling is triggered every time and hole is positioned correctly. Few things I don't like:
highlight
This is the new highlight. It's bigger and highlight entire element but you can see rough edges around rectangle.
2nd thing is that scroll animation is not smooth anymore. At least not all the time. Sometimes you can feel dropped frames in animation.
But that are small things. Apart from that everything else is fine for me.

@jwarby
Copy link
Owner

jwarby commented Apr 1, 2015

The size of the highlight is correct - it is supposed to be a bit larger than the element. I've no idea why you are getting those rough lines and choppy animation though... what browser are you using?

@Nogostradamus
Copy link
Author

I have tested it on both Firefox and Chrome (latest versions).
Choppy animation is not happening all the time. It might be my app. I have a lot of dynamic elements on screen. However, it didn't happen before. But it's not a big deal. Most of the time is fine.

Ps. Oh, I know why I have that rough edges. I forgot I have changed the background opacity in css file. That's why it doesn't fit the color on edges. My fault.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants