Fix default issue

[davidrenne]

This animation is not worth it as a default because of

https://github.com/zurb/foundation/blob/master/js/foundation/foundation.reveal.js#L324-L355

Is not tested very well for a default simple use case. Keep it simple.

[davidrenne]

Also I am not sure why end users configuring zurb foundation have to put hacks in our CSS like this:

.reveal-modal-bg{position:fixed;}
.reveal-modal{position:fixed;}

Just to get the modal window to not be busted most of the time?

I just want to go to bootstrap popup because zurb still doesnt have a really easy to use and non buggy default modal.

If we add those things, that could help the situation right?

[rafibomb]

Can you explain what issue this is supposed to resolve?

[davidrenne]

Recent code added to master is causing centering issues (in height) with the modal.

If you have an anchor farther down the page which calls data-reveal-id on a form. The form will never be centered because of code added by several authors the past few months.

https://github.com/zurb/foundation/blob/master/js/foundation/foundation.reveal.js#L324-L355

If you simply keep the default without all the fancy stuff, there will not be as many issues of people using this in the wild. I thought keep the base install simple.

Also I can barely even see how the animation by default is really adding that much value to user experience. I would prefer the base software not give headaches to users and suggest setting the default to no animation so it bypasses #L324-L355 incorrect assumptions of height and centering of the modal popup.

I really think that the reveal modal bg class should get what I specified with a fixed position, but that is for you guys to test and decide on.

[rafibomb]

Can you explain how this used to work vs how it works now? Testing it out, the modal open a certain distance from the top of the viewport no matter where the page is scrolled. Is that not expected behavior?

[davidrenne]

Try putting an anchor farther down the page where there is a scrollbar and
see if it centers in height for you. It doesnt in my case because of all
the extra javascript called.

On Fri, Mar 27, 2015 at 12:57 PM, Rafi notifications@github.com wrote:

Can you explain how this used to work vs how it works now? Testing it out,
the modal open a certain distance from the top of the viewport no matter
where the page is scrolled. Is that not expected behavior?

—
Reply to this email directly or view it on GitHub
#6415 (comment).

[jonathanmelville]

Please give extra scrutiny to this PR!

The last time a PR was accepted for Reveal (5.0), it completely broke it and we had to wait weeks for the next release to restore functionality. This is such a core component of Foundation, it would put out a great number of developers if it got broken again.

[davidrenne]

I guess we can start with someone recreating what I see. I can rollback a
build to a certain point to before I had this issue if need be if someone
wants to see the modal window jump all around in different directions based
on how far you are scrolled down when animation is turned on.

On Mon, Mar 30, 2015 at 10:52 AM, Jonathan notifications@github.com wrote:

Please give extra scrutiny to this PR!

The last time a PR was accepted for Reveal (5.0), it completely broke it
and we had to wait weeks for the next release to restore functionality.
This is such a core component of Foundation, it would put out a great
number of developers if it got broken again.

—
Reply to this email directly or view it on GitHub
#6415 (comment).

[jonathanmelville]

Can you create a CodePen that demonstrates this, or supply a link where this behavior is happening?

[gakimball]

@jonathanmelville What this guy said—a few of us were looking at this last week, and we need some help understanding it further.

[davidrenne]

Just close pull request. Not worth everyone's time. There are bugs in that new animation logic I referenced and I never saw how anything was even animated when I set the animation to null or when it was disabled.

—
Dave

On Mon, Mar 30, 2015 at 5:48 PM, Geoff Kimball notifications@github.com
wrote:

@jonathanmelville What this guy said—a few of us were looking at this last week, and we need some help understanding it further.

Reply to this email directly or view it on GitHub:
#6415 (comment)

[rafibomb]

Ok, we want to fix issue. We're looking at all this stuff as we re-build most of the JS plugins so it will be taken care of in the next version.