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(js): allow body scroll when detached mode responsively disabled #1251

Merged
merged 3 commits into from
May 14, 2024

Conversation

aldenquimby
Copy link
Contributor

@aldenquimby aldenquimby commented May 10, 2024

Summary

Result

  • See codesandbox example playground
  • I can now scroll the body in my local app after transitioning in/out of detached mode:
    algolia-result

Tech Notes

  • I also tried this, but it did not work, and I'm not exactly sure why:
    return () => {
      if (panelContainerElement.contains(panelElement)) {
        if (isDetached.value) {
          setIsModalOpen(false);
        } else {
          panelContainerElement.removeChild(panelElement);
        }
      }
    };

Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit b73fddb:

Sandbox Source
@algolia/autocomplete-example-github-repositories-custom-plugin Configuration
@algolia/autocomplete-example-instantsearch Configuration
@algolia/autocomplete-example-playground Configuration
@algolia/autocomplete-example-preview-panel-in-modal Configuration
@algolia/autocomplete-example-react-renderer Configuration
@algolia/autocomplete-example-starter-algolia Configuration
@algolia/autocomplete-example-starter Configuration
@algolia/autocomplete-example-reshape Configuration
@algolia/autocomplete-example-vue Configuration
@algolia/autocomplete-example-playground Issue #1250

Copy link

codesandbox-ci bot commented May 10, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 7b11332:

Sandbox Source
@algolia/autocomplete-example-github-repositories-custom-plugin Configuration
@algolia/autocomplete-example-instantsearch Configuration
@algolia/autocomplete-example-playground Configuration
@algolia/autocomplete-example-preview-panel-in-modal Configuration
@algolia/autocomplete-example-react-renderer Configuration
@algolia/autocomplete-example-starter-algolia Configuration
@algolia/autocomplete-example-starter Configuration
@algolia/autocomplete-example-reshape Configuration
@algolia/autocomplete-example-vue Configuration
@algolia/autocomplete-example-playground Issue #1250

@Haroenv
Copy link
Contributor

Haroenv commented May 13, 2024

Thanks a lot for this! Have you tried adding a test for this behaviour? Otherwise this should be good to merge

@aldenquimby
Copy link
Contributor Author

aldenquimby commented May 13, 2024

@Haroenv I just added a test that I think should work (stitched together by copying various other tests), but I am unable to get the test suite working locally. Could you either check it for me locally, or enable the CI suite to run and I can debug based on those logs? Thank you!

Haroenv
Haroenv previously approved these changes May 14, 2024
Copy link
Contributor

@Haroenv Haroenv left a comment

Choose a reason for hiding this comment

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

thanks!

@Haroenv Haroenv dismissed their stale review May 14, 2024 06:30

test doesn't pass yet, will fix

@Haroenv Haroenv merged commit 710f86b into algolia:next May 14, 2024
1 check was pending
@Haroenv
Copy link
Contributor

Haroenv commented May 14, 2024

Thanks a lot for your contribution! This is now in 1.17.1 :)

@aldenquimby
Copy link
Contributor Author

@Haroenv awesome, thank you for the assist!

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.

Body scroll should be restored when detached mode is responsively removed
2 participants