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

fix(useBodyScrollLock): avoid settings unrelated styles #1832

Merged
merged 1 commit into from
Oct 15, 2024

Conversation

StepanKirichenko
Copy link
Contributor

Fix the issue where useBodyScrollLock saved and then restored unrelated body styles.
Described in the issue #1724

@gravity-ui-bot
Copy link
Contributor

Preview is ready.

@gravity-ui-bot
Copy link
Contributor

Visual Tests Report is ready.

Copy link
Contributor

@korvin89 korvin89 Oct 10, 2024

Choose a reason for hiding this comment

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

I suggest you to write this in a little bit different way.

Add this things first:

import get from 'lodash/get';

// ...

const STORED_BODY_STYLE_KEYS = ['overflow', 'paddingRight', 'paddingBottom'] as const;
type StoredBodyStyleKeys = (typeof STORED_BODY_STYLE_KEYS)[number];
type StoredBodyStyle = Partial<Pick<CSSStyleDeclaration, StoredBodyStyleKeys>>;

// ...

function getStoredStyles(): StoredBodyStyle {
    const styles: StoredBodyStyle = {};

    for (const property of STORED_BODY_STYLE_KEYS) {
        styles[property] = get(document.body.style, property);
    }

    return styles;
}

Then use them:

// ...

function setBodyStyles() {
    const yScrollbarWidth = getYScrollbarWidth();
    const xScrollbarWidth = getXScrollbarWidth();
    const bodyPadding = getBodyComputedPadding();

+    storedBodyStyle = getStoredStyles();
-      storedBodyStyle = {
-          overflow: document.body.style.overflow,
-         paddingRight: document.body.style.paddingRight,
-          paddingBottom: document.body.style.paddingBottom,
-     };

    document.body.style.overflow = 'hidden';

    if (yScrollbarWidth) {
        document.body.style.paddingRight = `${bodyPadding.right + yScrollbarWidth}px`;
    }
    if (xScrollbarWidth) {
        document.body.style.paddingBottom = `${bodyPadding.bottom + xScrollbarWidth}px`;
    }
}

// ...

function restoreBodyStyles() {
-    for (const property of ['overflow', 'paddingRight', 'paddingBottom'] as const) {
+    for (const property of STORED_BODY_STYLE_KEYS) {
        const storedProperty = storedBodyStyle[property];
        if (storedProperty) {
            document.body.style[property] = storedProperty;
        } else {
            document.body.style.removeProperty(property);
        }
    }
}

It will be more convenient (there will be only one typed list with properties).

@StepanKirichenko StepanKirichenko merged commit 9d7fd36 into main Oct 15, 2024
6 checks passed
@StepanKirichenko StepanKirichenko deleted the fix-use-body-scroll-lock-stored-styles branch October 15, 2024 12:25
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.

3 participants