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

[SAGE-518] Modal - remove ability to scroll background when modal is open #1559

Merged
merged 3 commits into from
Aug 22, 2022

Conversation

QuintonJason
Copy link
Contributor

@QuintonJason QuintonJason commented Aug 15, 2022

Description

  • remove ability to scroll background when modal is open

Screenshots

Before After
modalScrollBefore modalScroll

Testing in sage-lib

Visit the Modal page and verify that .sage-page--has-open-modal is present in the <body> when a modal is open to prevent background scrolling.

Testing in kajabi-products

  1. (HIGH) Modal - adds class to <body> when modal a modal is open.
    • Creation Wizard
    • Pipelines Blueprint pages

Related

Closes SAGE-518

@QuintonJason QuintonJason added the improvement Improve on existing work label Aug 15, 2022
@QuintonJason QuintonJason self-assigned this Aug 15, 2022
@QuintonJason QuintonJason marked this pull request as ready for review August 16, 2022 13:50
@QuintonJason QuintonJason requested review from a team August 16, 2022 13:50
@goodwinchris
Copy link
Contributor

I'm seeing the sage-page--has-open-modal class in storybook on page load

@@ -36,6 +36,11 @@ export const Modal = ({
}
);

useEffect(() => {
document.body.className = 'sage-page--has-open-modal';
return () => { document.body.className = ''; };
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good job...resetting the class when the Modal unmounts :chefkiss:

@QuintonJason
Copy link
Contributor Author

I'm seeing the sage-page--has-open-modal class in storybook on page load

I'll look into this

@QuintonJason QuintonJason force-pushed the SAGE-518/qj-modal-scroll-background-when-open branch from 01085ec to 02142b3 Compare August 18, 2022 17:57
Copy link
Member

@pixelflips pixelflips left a comment

Choose a reason for hiding this comment

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

LGTM! Working as expected. Nice work! 🔥

I also enabled the bridge and then ran the spec that was causing the headache with the last iteration and it's passing as well.

May want to add the pipelines blueprint pages as an area that QE should check as well just in case.

@pixelflips pixelflips requested a review from a team August 18, 2022 18:09
@@ -23,6 +23,6 @@ $-banner-height-offset: map-get($sage-banner-heights, default);
margin-bottom: sage-spacing();
}

.sage-page__has-open-modal {
.sage-page--has-open-modal {
Copy link
Contributor

Choose a reason for hiding this comment

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

😍

Copy link
Collaborator

@ju-Skinner ju-Skinner left a comment

Choose a reason for hiding this comment

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

:shipit:

@QuintonJason QuintonJason force-pushed the SAGE-518/qj-modal-scroll-background-when-open branch from 60c887f to d1be6eb Compare August 19, 2022 14:07
@QuintonJason QuintonJason merged commit 90a6b82 into develop Aug 22, 2022
@QuintonJason QuintonJason deleted the SAGE-518/qj-modal-scroll-background-when-open branch August 22, 2022 15:44
@QuintonJason QuintonJason mentioned this pull request Aug 22, 2022
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improve on existing work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants