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

refactor(layout): Refactor layout structure to improve Navbar positioning & child component #661

Merged
merged 2 commits into from
Jan 25, 2025

Conversation

Allan2000-Git
Copy link
Contributor

@Allan2000-Git Allan2000-Git commented Jan 25, 2025

User description

Description

Simplify Layout component and improve content distribution

  • Removed static height
  • Implemented flex column layout for better space utilization
  • Added overflow-auto to content area for proper scrolling

Fixes #659

Dependencies

No new dependencies introduced.

Future Improvements

N/A

Mentions

@rajdip-b

Screenshots of relevant screens

Before
before change

After
after-change

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, Bug fix


Description

  • Refactored layout structure for better Navbar positioning.

  • Adjusted content area to enable proper scrolling.

  • Removed static height in ProfilePage for dynamic layout.

  • Improved child component overflow handling in AppLayout.


Changes walkthrough 📝

Relevant files
Enhancement
layout.tsx
Refactor layout for scrolling and Navbar positioning         

apps/platform/src/app/(main)/layout.tsx

  • Refactored layout to use a flex column structure.
  • Added overflow-auto to content area for proper scrolling.
  • Ensured Navbar and child components are dynamically positioned.
  • +7/-3     
    Bug fix
    page.tsx
    Adjust `ProfilePage` layout for dynamic height                     

    apps/platform/src/app/(main)/settings/@profile/page.tsx

  • Removed static height from ProfilePage layout.
  • Simplified layout for better dynamic height handling.
  • +1/-1     

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ✅

    659 - PR Code Verified

    Compliant requirements:

    • Fix scrolling issue in Profile tab section where users cannot scroll to bottom due to fixed navbar
    • Scrollable area should adjust for navbar height
    • All content should be accessible via scrolling

    Requires further human verification:

    • Verify fix works properly on Windows/Chrome browser
    • Verify scrolling behavior works correctly with different screen sizes
    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Responsive Layout

    Verify that the new flex layout and overflow handling works correctly across different viewport sizes and content lengths

    <div className="m-8 h-dvh overflow-clip rounded-[1.125rem] bg-[#161819] md:h-[90vh] md:w-[90vw] 2xl:h-[93vh]">
      <div className="flex flex-col h-full">
          <Navbar />
          <div className="flex-1 overflow-auto p-4">
              {children}
          </div>
      </div>

    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    General
    Prevent content overflow issues

    Add a max-height constraint to prevent potential overflow issues in the content
    area, especially for large screens or when content grows dynamically.

    apps/platform/src/app/(main)/layout.tsx [16-18]

    -<div className="flex-1 overflow-auto p-4">
    +<div className="flex-1 overflow-auto p-4 max-h-[calc(100vh-theme(spacing.16))]">
         {children}
     </div>
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: The suggestion addresses a critical UI issue by preventing content overflow on large screens, which aligns with the PR's focus on proper scrolling and layout management. The max-height calculation ensures consistent behavior across different viewport sizes.

    8
    Improve accessibility with ARIA labels

    Add aria-label to the main content area for better accessibility and screen reader
    support.

    apps/platform/src/app/(main)/layout.tsx [13]

    -<div className="m-8 h-dvh overflow-clip rounded-[1.125rem] bg-[#161819] md:h-[90vh] md:w-[90vw] 2xl:h-[93vh]">
    +<div className="m-8 h-dvh overflow-clip rounded-[1.125rem] bg-[#161819] md:h-[90vh] md:w-[90vw] 2xl:h-[93vh]" aria-label="Main content area">
    • Apply this suggestion
    Suggestion importance[1-10]: 4

    Why: While adding ARIA labels improves accessibility, the impact is moderate since the main content area is already semantically structured. The suggestion enhances screen reader support but doesn't address critical functionality issues.

    4

    @rajdip-b rajdip-b merged commit 31067f3 into keyshade-xyz:develop Jan 25, 2025
    4 checks passed
    rajdip-b pushed a commit that referenced this pull request Jan 25, 2025
    ## [2.10.0-stage.5](v2.10.0-stage.4...v2.10.0-stage.5) (2025-01-25)
    
    ### 🐛 Bug Fixes
    
    * **platform:** Refactor layout structure to improve Navbar positioning & child component ([#661](#661)) ([31067f3](31067f3))
    @rajdip-b
    Copy link
    Member

    🎉 This PR is included in version 2.10.0-stage.5 🎉

    The release is available on GitHub release

    Your semantic-release bot 📦🚀

    @Allan2000-Git Allan2000-Git deleted the refactor/main-layout branch January 25, 2025 15:24
    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.

    PLATFORM: Unable to Scroll to Bottom of the Profile tab Section
    2 participants