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

Jcatania/560 welcome modal bootstrap #575

Conversation

jamiecatania
Copy link
Collaborator

@NealHumphrey It didn't actually require much refactoring at all, and buttons appear to be working. Let me know if anything else is needed on this!

@NealHumphrey
Copy link
Collaborator

@jamiecatania Unfortunately it doesn't look like the core issue is solved yet. The main problem was that if the screen height was too small, i.e. shorter than the height of the modal, the buttons were locked off-screen with no way to scroll to them:

image

In the semantic ui modal, calling the 'resize' at the end of the page load would fix this by adding a page scroll bar and letting you scroll the modal while the background was static. Looks like we can't do that here.

Can you look into that aspect of it? On nice high-res screens it's not an issue but on some screens it still will be.

@jamiecatania
Copy link
Collaborator Author

jamiecatania commented Sep 21, 2017

@NealHumphrey - It appears that adding overflow-y: auto to the modal provides the desired scrolling effect at a variety of low resolutions. I've tested in chrome and firefox at resolutions ranging from 480 x 800 all the way up to 2560 x 1440. The scrollbar is showing and allowing access to the buttons in all cases. Also, the scrolling action doesn't appear to effect the map/other elements under the modal.

@NealHumphrey
Copy link
Collaborator

@jamiecatania so I was trying to put in a quick-fix hack so that the deploy would be ok on this issue. Turns out I stumbled into a good-enough-for now hack. If you call the 'refresh' function on the semantic ui modal twice in a row, it resolves the scrolling issue. The first refresh moves the modal up the page slightly, and the seocnd one realizes that in it's new location it still doesn't fit so adds the scroll.

I've put this into the current dev and master branches and it's up on the live site. Not perfect b/c there's still a slight delay, but the whole page is still loading so I'm ok with just using this hack to get it usable rather than going down another debugging rabbit hole.

@NealHumphrey
Copy link
Collaborator

@jamiecatania Looks like we wrote at the same time. If your css change works I'm happy to use that instead of my hack. Looks like you've updated the PR. I'll test it out, either tomorrow or Monday

@NealHumphrey NealHumphrey merged commit fc3e9fd into focusconsulting:dev Sep 21, 2017
@NealHumphrey
Copy link
Collaborator

@jamiecatania overflow-y fixed it, so simple! Wonder if that would have worked on the semantic-ui one. Anyways, we're merged in and good to go on this issue. Thanks!

@jamiecatania
Copy link
Collaborator Author

jamiecatania commented Sep 21, 2017 via email

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

Successfully merging this pull request may close these issues.

2 participants