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(web): Horizontal Scrolling issue on the website #440

Conversation

Samaresh-Das
Copy link
Contributor

@Samaresh-Das Samaresh-Das commented Sep 17, 2024

User description

Description

The website had a horizontal scroll bar when changing to different screen sizes. The tailwind breakpoints were not used properly in my opinion. So I added some more breakpoints so it becomes more responsive as some cards in the page are pushing the beyond the View port. There were two options, either tweak the width of the cards or change the grid layout. Changing the width of the cards making elements look bad in other sections of the page and is overall not a good idea as card is a reusable component. So I changed the parent container div layout. On desktop screen it will be 3, on tablet size it will be 2. I didn't touched the mobile responsive design.

Video-

Recording.2024-09-17.1629061.mp4

Fixes #437

Dependencies

Mention any dependencies/packages used

Future Improvements

Mention any improvements to be done in future related to any file/feature

Mentions

Mention and tag the people

Screenshots of relevant screens

Add screenshots of relevant screens

Developer's checklist

  • My PR follows the style guidelines of this project
  • I have performed a self-check on my work

If changes are made in the code:

  • I have followed the coding guidelines
  • My changes in code generate no new warnings
  • My changes are breaking another fix/feature of the project
  • I have added test cases to show that my feature works
  • I have added relevant screenshots in my PR
  • There are no UI/UX issues

Documentation Update

  • This PR requires an update to the documentation at docs.keyshade.xyz
  • I have made the necessary updates to the documentation, or no documentation changes are required.

PR Type

enhancement


Description

  • Enhanced the grid layout across multiple components to address horizontal scrolling issues and improve responsiveness.
  • Adjusted grid columns for different screen sizes, ensuring better layout on desktop, tablet, and mobile devices.
  • Added conditional rendering in LifeEasySection to optimize display based on screen size.

Changes walkthrough 📝

Relevant files
Enhancement
index.tsx
Update grid layout for responsive design                                 

apps/web/src/components/colabEasy/index.tsx

  • Modified grid layout to improve responsiveness.
  • Adjusted grid columns for different screen sizes.
  • +1/-1     
    index.tsx
    Enhance grid layout for better responsiveness                       

    apps/web/src/components/lifeEasySection/index.tsx

  • Changed grid layout to improve responsiveness.
  • Added conditional rendering for different screen sizes.
  • +26/-2   
    index.tsx
    Improve grid layout for responsive design                               

    apps/web/src/components/secretSection/index.tsx

  • Updated grid layout for improved responsiveness.
  • Adjusted grid columns for various screen sizes.
  • +1/-1     

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Code Duplication
    The component for 'Command Line Interface' and 'Snapshot' is duplicated for different screen sizes. Consider refactoring to avoid repetition.

    Copy link
    Contributor

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Performance
    Extract card rendering logic into a memoized component to optimize performance

    To improve performance and avoid unnecessary re-renders, consider extracting the
    card rendering logic into a separate component and memoizing it. This is especially
    useful if the cardData array is large or frequently updated.

    apps/web/src/components/secretSection/index.tsx [63-67]

    +const SecretCard = React.memo(({ heading, description, svg }) => (
    +  <Card>
    +    {svg}
    +    <div className="p-6">
    +      <h3 className="text-lg font-medium">{heading}</h3>
    +      <span className="text-base text-[#9394A1]">{description}</span>
    +    </div>
    +  </Card>
    +));
    +
     <div className="grid gap-3 md:grid-cols-2 xl:grid-cols-3 2xl:gap-9">
    -  {cardData.map((card, index) => {
    -    const { heading, description, svg } = card
    -    return (
    -      // eslint-disable-next-line react/no-array-index-key -- safe
    -      <Card key={index}>
    -        {svg}
    -        <div className="p-6">
    -          <h3 className="text-lg font-medium">{heading}</h3>
    -          <span className="text-base text-[#9394A1]">{description}</span>
    -        </div>
    -      </Card>
    -    )
    -  })}
    +  {cardData.map((card, index) => (
    +    <SecretCard key={card.heading} {...card} />
    +  ))}
     </div>
     
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: Extracting the card rendering logic into a memoized component is a strong suggestion for optimizing performance, especially if cardData is large or frequently updated. This change can significantly reduce unnecessary re-renders, improving the application's efficiency.

    9
    Maintainability
    Refactor the component to use a single set of card components with responsive classes to reduce code duplication

    Instead of duplicating the card components for different screen sizes, consider
    using a single set of components with responsive classes. This approach reduces code
    duplication and improves maintainability.

    apps/web/src/components/lifeEasySection/index.tsx [25-107]

     <div className="grid gap-9 md:grid-cols-2 xl:grid-cols-3">
    -  <div className="grid gap-9">
    -    <Card>
    -      <StandardKitSVG />
    +  {cards.map((card, index) => (
    +    <Card key={index} className={index > 0 ? 'hidden md:block' : ''}>
    +      {card.svg}
           <div className="p-6">
    -        ...
    +        <h3 className="text-lg font-medium">{card.title}</h3>
    +        <span className="text-base text-[#9394A1]">{card.description}</span>
           </div>
         </Card>
    -    ...
    -  </div>
    -  <div className="hidden md:grid xl:hidden">
    -    <Card>
    -      ...
    -    </Card>
    -  </div>
    -  <div className="hidden md:grid xl:hidden">
    -    <Card>
    -      ...
    -    </Card>
    -  </div>
    +  ))}
     </div>
     
    Suggestion importance[1-10]: 8

    Why: This suggestion effectively reduces code duplication and enhances maintainability by using a single set of components with responsive classes. It addresses a significant issue in the code structure, making it more efficient and easier to manage.

    8
    Enhancement
    Implement a more flexible and responsive grid layout using CSS Grid's auto-fit feature

    Consider using a more flexible grid system that adapts to different screen sizes
    without the need for multiple breakpoints. You could use CSS Grid's auto-fit or
    auto-fill with minmax() to create a responsive layout that automatically adjusts the
    number of columns based on available space.

    apps/web/src/components/colabEasy/index.tsx [25]

    -<div className="auto-cols-min gap-5 space-y-5 md:grid md:grid-cols-2 md:space-y-0 xl:grid-cols-3">
    +<div className="grid gap-5 grid-cols-1 sm:grid-cols-auto-fit sm:grid-cols-[repeat(auto-fit,minmax(250px,1fr))]">
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: The suggestion to use CSS Grid's auto-fit feature can improve the responsiveness and flexibility of the layout, reducing the need for multiple breakpoints. However, it is not a critical change and may require additional testing to ensure it behaves as expected across all screen sizes.

    7

    @rajdip-b
    Copy link
    Member

    @kriptonian1 could you review this and get it merged?

    Copy link
    Contributor

    @kriptonian1 kriptonian1 left a comment

    Choose a reason for hiding this comment

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

    LGTM

    @rajdip-b rajdip-b force-pushed the fix_Horizontal_Scrolling_Issue_on_the_Website branch from 0e0515a to eb4996b Compare September 18, 2024 04:16
    @rajdip-b rajdip-b changed the title fix(UI): Horizontal Scrolling Issue on the Website fix(web): Horizontal Scrolling issue on the website Sep 18, 2024
    @rajdip-b rajdip-b merged commit 655177b into keyshade-xyz:develop Sep 18, 2024
    6 checks passed
    @m7md-Y-3mra
    Copy link
    Contributor

    m7md-Y-3mra commented Sep 18, 2024

    Hi @rajdip-b
    I wanted to bring to your attention a problem I’ve encountered related to the recent changes merged in this PR. There is a horizontal scrolling issue when the screen width is between 762px and 852px.

    VID_._.mp4

    Kiranchaudhary537 pushed a commit to Kiranchaudhary537/keyshade that referenced this pull request Oct 13, 2024
    rajdip-b pushed a commit that referenced this pull request Oct 24, 2024
    ## [2.6.0](v2.5.0...v2.6.0) (2024-10-24)
    
    ### 🚀 Features
    
    * **api:**  Add icon and remove description field from workspace ([#435](#435)) ([a99c0db](a99c0db))
    * **api-client:** Added workspace-membership and related tests ([#452](#452)) ([6a1c091](6a1c091))
    * **api-client:** Create controller for User module ([#484](#484)) ([f9d8e83](f9d8e83))
    * **api:** Add prod env schema in env file ([#436](#436)) ([21c3004](21c3004))
    * **api:** Add resend otp implementation ([#445](#445)) ([4dc6aa1](4dc6aa1))
    * **api:** Fetch total count of environments, [secure]s and variables in project ([#434](#434)) ([0c9e50a](0c9e50a))
    * **api:** Replace `projectId` with `name` and `slug` in workspace-role response.  ([#432](#432)) ([af06071](af06071))
    * **cli:** Add functionality to operate on Secrets ([#504](#504)) ([1b4bf2f](1b4bf2f))
    * **cli:** Add project command ([#451](#451)) ([70448e1](70448e1))
    * **cli:** Add workspace operations ([#441](#441)) ([ed38d22](ed38d22))
    * **cli:** implement commands to get, list, update, and delete, workspace roles ([#469](#469)) ([957ea8d](957ea8d))
    * **cli:** Implemented pagination support ([#453](#453)) ([feb1806](feb1806))
    * **cli:** Secret scan ([#438](#438)) ([85cb8ab](85cb8ab))
    * **cli:** Update environment command outputs ([f4af874](f4af874))
    * **platform:** Clearing email field after waitlisting the user email ([#481](#481)) ([256d659](256d659))
    * Remove project IDs from workspace role export data and update tests ([#448](#448)) ([8fdb328](8fdb328))
    * **web:** Configured extra check for waitlisted users already in the list and created toast message for them ([#492](#492)) ([2ddd0ef](2ddd0ef))
    * **web:** show the toast only when email add successfully ([#490](#490)) ([783c411](783c411))
    
    ### 🐛 Bug Fixes
    
    * **api,api-client:** Add environmentSlug in multiple places across the variable module ([#468](#468)) ([d970aff](d970aff))
    * **api:** Replace the id with slug in the global-search service ([#455](#455)) ([74804b1](74804b1))
    * **platform:** Fixed duplicate Google Logo UI fix  ([#450](#450)) ([fb0d6f7](fb0d6f7))
    * resolve footer website name cut-off or overlap issue ([#444](#444)) ([fe03ba2](fe03ba2))
    * **web:** Horizontal Scrolling issue on the website ([#440](#440)) ([655177b](655177b))
    
    ### 📚 Documentation
    
    * Add documentation for environment in CLI ([#462](#462)) ([dad7394](dad7394))
    * Add documentation for project in CLI ([#466](#466)) ([341fb32](341fb32))
    * Add documentation for scan in CLI ([#461](#461)) ([72281e6](72281e6))
    * Add documentation for workspace command ([#464](#464)) ([4aad8a2](4aad8a2))
    * Add instructions for resetting the local Prisma database ([#495](#495)) ([#501](#501)) ([b07ea17](b07ea17))
    * Added docker support documentation ([#465](#465)) ([bc04be4](bc04be4))
    * Added documentation for running the platform ([#473](#473)) ([8b8386b](8b8386b))
    * Added missing mappings to pages ([5de9fd8](5de9fd8))
    * Fix Documentation Hyperlink and update expired Discord invite link ([#496](#496)) ([5a10e39](5a10e39))
    * Updated CLI docs ([#460](#460)) ([c7e0f13](c7e0f13))
    
    ### 🔧 Miscellaneous Chores
    
    * Add more logging to Sentry init ([#470](#470)) ([de4925d](de4925d))
    * **api:** Optimise API docker image size ([#360](#360)) ([ea40dc1](ea40dc1))
    * **api:** Updated lockfile ([a968e78](a968e78))
    * **CI:** Add [secure] scan validation ([f441262](f441262))
    * **cli:** Update controller invocation in environment commands ([#477](#477)) ([596bd1a](596bd1a))
    * Minor changes to variables ([fe01ca6](fe01ca6))
    * **[secure]-scan:** Failing lint issues ([#507](#507)) ([48f45df](48f45df))
    * **[secure]-scan:** Formatted files ([5884833](5884833))
    * Update .env.example ([70ad4f7](70ad4f7))
    * Updated scripts ([9eb76a7](9eb76a7))
    * **web:** email validation ([#487](#487)) ([e8e737a](e8e737a))
    @rajdip-b
    Copy link
    Member

    🎉 This PR is included in version 2.6.0 🎉

    The release is available on GitHub release

    Your semantic-release bot 📦🚀

    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.

    Horizontal Scrolling Issue on the Website
    4 participants