Skip to content

Conversation

@p-shannon
Copy link

Fixes #7 and elementary/website#1255

I don't know how to test this so please tell me how to do so, or test it yourself and confirm the proposed changes are sufficient. Thank you!

@lewisgoddard
Copy link
Member

To test you should be able to just open the index.html file in the repository and click any modal button there.

I think we're going to have to expressly include a container though, styling the modals normal parent will probably break some other styling. It's probably best if we have a container that is explicitly set as 100% width and 100% height to center in.

@p-shannon
Copy link
Author

Alright, tested it and everything and it's good. Let me know if anything else needs doing (differently).

@p-shannon p-shannon changed the title [DON'T MERGE] Changes centering calculations for modals [Ready] Changes centering calculations for modals Feb 14, 2018
@lewisgoddard
Copy link
Member

I'm closing this in favour of #13 where I've done basically the same thing by moving the existing overlay around and using that for positioning instead of creating a whole new element. Thanks for the help!

@p-shannon
Copy link
Author

No problem. I actually tried to move it to the existing overlay, but I couldn't figure out how to resolve the resulting opacity issue. I'll look at your implementation to see how it was done.

@p-shannon p-shannon deleted the patch-1 branch February 15, 2018 03:46
@lewisgoddard
Copy link
Member

I passed the opacity into an rgba background so that it wouldn't affect children like an actual opacity style would.

@p-shannon
Copy link
Author

Yep, just saw the code now. I thought about that, but then I realized I'd have to figure out how exactly you handled the fade out. I also see you refactored the relevant bits significantly, even changing the overlay from a class to an ID, which isn't something I could have done.

@lewisgoddard
Copy link
Member

Yeah, I ended up just breaking the fadeOut in favor of CSS Transitions. jQuery was getting some pretty poor framerates over on elementary.

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

Successfully merging this pull request may close these issues.

2 participants