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

Update the modal component to prevent the page from scrolling behind it when open #805

Merged
merged 2 commits into from
Apr 3, 2019

Conversation

alex-ju
Copy link
Contributor

@alex-ju alex-ju commented Mar 27, 2019

govuk-frontend v2.8.0 introduces an overflow-y: scroll for .govuk-template that needs to be overwritten when the modal opens otherwise the content will scroll behind the modal. alphagov/govuk-frontend#1230

@benthorner benthorner temporarily deployed to govuk-publishing-compon-pr-805 March 27, 2019 17:24 Inactive
@alex-ju alex-ju changed the title Update modal component to deal with overflow html Update the modal component to prevent the page from scrolling behind it when open Mar 27, 2019
@alex-ju alex-ju requested review from aliuk2012 and andysellick and removed request for aliuk2012 March 28, 2019 10:02
@benthorner benthorner temporarily deployed to govuk-publishing-compon-pr-805 March 28, 2019 10:07 Inactive
Copy link
Contributor

@aliuk2012 aliuk2012 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@andysellick andysellick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused by this change, I think it could do with some more explanation.

govuk-frontend v2.8.0 introduces an overflow-y: scroll for govuk-template that needs to be overwritten when the modal opens otherwise the content will scroll behind the modal. alphagov/govuk-frontend#1230
@alex-ju
Copy link
Contributor Author

alex-ju commented Apr 3, 2019

@andysellick are you happy with me to merge this or shall we have a chat about it beforehand?

@andysellick
Copy link
Contributor

@alex-ju thanks for asking. I think I'm happy with this change, although I'm still nervous about the non-isolation aspects of the component as a whole. But, this is something we need, so that's fine.

@alex-ju
Copy link
Contributor Author

alex-ju commented Apr 3, 2019

@andysellick I share your worry about isolation. It's the only solution I've seen at the moment to deal with the blur effect. I'm looking into removing it if I find a good alternative that doesn't involve 'communicating' with the template.

@alex-ju alex-ju merged commit 30e49c8 into master Apr 3, 2019
@alex-ju alex-ju deleted the update-modal-component branch April 3, 2019 12:06
@alex-ju alex-ju mentioned this pull request Apr 4, 2019
alex-ju added a commit to alphagov/content-publisher that referenced this pull request Apr 17, 2019
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.

4 participants