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

Fixed main page scroll issue while popup open. #938

Closed
wants to merge 1 commit into from

Conversation

prateekkalra
Copy link
Contributor

@prateekkalra prateekkalra commented Oct 16, 2023

Made main page unscrollable made popup is visible.
/claim #763

Summary by CodeRabbit

  • Refactor: Improved the user experience of the popup feature in our application. Now, when the bot is opened or closed, the page's scrolling behavior is more controlled, providing a smoother interaction for users. This change enhances the overall usability of the bot feature by ensuring a more seamless transition between different states of the application.

@coderabbitai
Copy link

coderabbitai bot commented Oct 16, 2023

Walkthrough

The changes introduced in this pull request primarily focus on improving the modularity and reusability of the code in the Popup component. The overflow manipulation logic, which was previously scattered in the openBot and closeBot functions, has now been encapsulated in a separate function named setOverflow.

Changes

File Summary
.../popup/components/Popup.tsx A new function setOverflow has been added to control the overflow style property of the document.body. This function is now being called in the openBot and closeBot functions, replacing the direct assignments to document.body.style.overflow. This enhances the code's modularity and reusability.

Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • Mention @coderabbitai in any review comment for bot assistance.
  • Note: Review comments are made on code diffs or files, not on the PR overview.

Pause Incremental Reviews

  • Insert @coderabbitai: ignore in the PR description to halt the bot's ongoing reviews. Remove the line to resume.
  • Additionally, enter @coderabbitai pause to halt the bot's ongoing reviews as any issue or review comment. To
    resume reviews, enter @coderabbitai resume as any issue or review comment.

@vercel
Copy link

vercel bot commented Oct 16, 2023

@prateekkalra is attempting to deploy a commit to the Typebot Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 1

Commits Files that changed from the base of the PR and between 1d0aab7 and 66b1595.
Files selected for processing (1)
  • packages/embeds/js/src/features/popup/components/Popup.tsx (2 hunks)
Additional comments (Suppressed): 1
packages/embeds/js/src/features/popup/components/Popup.tsx (1)
  • 92-104: The changes in openBot and closeBot functions are correct and improve the code's modularity by using the setOverflow function to manage the overflow property of the document.body. This change doesn't alter the logic of the functions and should work as expected.

@@ -41,6 +41,8 @@ export const Popup = (props: PopupProps) => {
popupProps.isOpen ?? false
)

const setOverflow = value => document.body.style.setProperty('overflow', value, 'important');
Copy link

Choose a reason for hiding this comment

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

The setOverflow function is a good abstraction for setting the overflow property of the document.body. However, it's recommended to check if document and document.body exist before accessing their properties. This is to avoid potential errors in environments where these objects might not be available, such as server-side rendering.

- const setOverflow = value => document.body.style.setProperty('overflow', value, 'important');
+ const setOverflow = value => {
+   if (typeof document !== 'undefined' && document.body) {
+     document.body.style.setProperty('overflow', value, 'important');
+   }
+ };

@baptisteArno
Copy link
Owner

Solved first with #937 🙏

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