Skip to content
This repository has been archived by the owner on Sep 7, 2024. It is now read-only.

Remove the scrollbar from the subscriber section #823

Closed
wants to merge 1 commit into from
Closed

Remove the scrollbar from the subscriber section #823

wants to merge 1 commit into from

Conversation

HimanshuMahto
Copy link

@HimanshuMahto HimanshuMahto commented Oct 26, 2023

Fixes Issue

This PR fixes the following issues:

closes #741

Changes proposed

I made the changes in the Newsletter.jsx with which the scrollbar showing in the mobile phone won't show anymore.

Here comes all the changes proposed through this PR

Check List (Check all the boxes which are applicable)

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • All new and existing tests passed.
  • This PR does not contain plagiarized content.
  • The title of my pull request is a short description of the requested changes.

Screenshots

Add all the screenshots which support your changes

Summary by CodeRabbit

  • Improvement: Enhanced the responsiveness of the NewsLetter section on our website. The section now dynamically adjusts its size based on your screen size, providing a better viewing experience across different devices. This update ensures that the content is always displayed in an optimal manner, regardless of how you resize your browser window.

@coderabbitai
Copy link

coderabbitai bot commented Oct 26, 2023

Walkthrough

The introduced changes enhance the responsiveness of the NewsLetter component. The component now dynamically adjusts the iframe height based on the window width, ensuring a seamless user experience across different screen sizes.

Changes

File Summary
src/components/layout/JoinUs/NewsLetter.jsx Added an event listener for the resize event. The listener calls the updateIframeHeight function, which adjusts the iframe height based on the window width. A cleanup function removes the event listener when the component is unmounted.

🐇

"In the land of code, where the shadows lie,

A rabbit hopped, with a twinkle in its eye.

With each resize, the iframe did dance,

Thanks to the rabbit's clever enhancement. 🎉"


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • If you reply to a review comment from CodeRabbit, the bot will automatically respond.
  • To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
  • Note: Review comments are made on code diffs or files, not on the PR overview.
  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai help to get help.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.json

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Woah! This is your first Pull Request in our community, Congratulations 🎉 and thank you for your contribution! 🚀 The team will review it on availability 💫. In the meantime join our Discord server and checkout our YouTube channel.

@HimanshuMahto
Copy link
Author

Hey @Roshan-Horo @anongorax @apoorv-on-git @itshypon Please review this PR.

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

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between f93c430 and 889c5b1.
Files selected for processing (1)
  • src/components/layout/JoinUs/NewsLetter.jsx (1 hunks)

Comment on lines +7 to +25
const updateIframeHeight = () => {
const windowWidth = window.innerWidth;

if (windowWidth < 900) {
setIframeHeight(830);
} else if (windowWidth < 1250) {
setIframeHeight(700);
} else {
setIframeHeight(670);
}
};

updateIframeHeight();

window.addEventListener('resize', updateIframeHeight);

return () => {
window.removeEventListener('resize', updateIframeHeight);
};
Copy link

Choose a reason for hiding this comment

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

The new hunk introduces an event listener for the resize event to dynamically adjust the iframe height based on the window width. This is a good approach to enhance the responsiveness of the component. The cleanup function to remove the event listener when the component is unmounted is also correctly implemented, which is crucial to prevent potential memory leaks. However, there is a potential performance issue here. The resize event can fire many times per second during a "resize" operation, leading to high processing overhead if the event handler does much work. In this case, the event handler updateIframeHeight calls setIframeHeight which triggers a re-render of the component. To mitigate this, consider debouncing the updateIframeHeight function to limit the rate at which it's invoked.

+ import { debounce } from 'lodash';

  useEffect(() => {
    const updateIframeHeight = debounce(() => {
      const windowWidth = window.innerWidth;

      if (windowWidth < 900) {
        setIframeHeight(830);
      } else if (windowWidth < 1250) {
        setIframeHeight(700);
      } else {
        setIframeHeight(670);
      }
    }, 250); // Debounce time in milliseconds

    updateIframeHeight();

    window.addEventListener('resize', updateIframeHeight);

    return () => {
      window.removeEventListener('resize', updateIframeHeight);
    };
  }, []);
Committable suggestion (Beta)
Suggested change
const updateIframeHeight = () => {
const windowWidth = window.innerWidth;
if (windowWidth < 900) {
setIframeHeight(830);
} else if (windowWidth < 1250) {
setIframeHeight(700);
} else {
setIframeHeight(670);
}
};
updateIframeHeight();
window.addEventListener('resize', updateIframeHeight);
return () => {
window.removeEventListener('resize', updateIframeHeight);
};
import { debounce } from 'lodash';
const NewsLetter = () => {
const [iframeHeight, setIframeHeight] = useState(670);
useEffect(() => {
const updateIframeHeight = debounce(() => {
const windowWidth = window.innerWidth;
if (windowWidth < 900) {
setIframeHeight(830);
} else if (windowWidth < 1250) {
setIframeHeight(700);
} else {
setIframeHeight(670);
}
}, 250); // Debounce time in milliseconds
updateIframeHeight();
window.addEventListener('resize', updateIframeHeight);
return () => {
window.removeEventListener('resize', updateIframeHeight);
};
}, []);
return (
// Rest of the component
);
};
export default NewsLetter;

@HimanshuMahto HimanshuMahto closed this by deleting the head repository Nov 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

subscribe form is having scrollbar on devices below 452px inside a section [bug]
1 participant