Skip to content

fix: black gap on on cloud in sidebar (#7383) #7427

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

Merged
merged 2 commits into from
Apr 28, 2025

Conversation

gabber235
Copy link
Contributor

@gabber235 gabber235 commented Mar 25, 2025

Summary

Fixed an issue in the side navigation where a black gap would appear when there were many primary navigation items. The solution implements a flexbox layout approach for both primary and secondary navigation items, ensuring proper vertical spacing and eliminating the unwanted gap at the bottom of the navigation panel.

Related Issues / PR's

#7383

Screenshots

Screenshots are illustrated by duplicating the nav items
Before:
SCR-20250325-ifpn

After:
SCR-20250325-ignp

Affected Areas and Manually Tested Areas

  • Component: SideNav component

  • File modified: frontend/src/container/SideNav/SideNav.styles.scss

  • Changes:

    • Replaced fixed positioning with flexbox layout for better space distribution
    • Added flex column direction to both primary and secondary nav items
    • Set flex properties to ensure proper spacing and growth behavior
    • Removed hardcoded max-height constraints that were causing the gap
    • Removed position:fixed, bottom:0, and left:0 properties that were contributing to the layout issue
  • Testing: Manually tested with various numbers of navigation items to ensure the gap no longer appears and that all items remain accessible.


Important

Fixes black gap in sidebar by switching to flexbox layout in SideNav.styles.scss, ensuring proper spacing and accessibility.

  • Behavior:
    • Fixes black gap issue in sidebar by implementing flexbox layout in SideNav.styles.scss.
    • Ensures proper vertical spacing and eliminates unwanted gap at bottom of navigation panel.
  • Layout Changes:
    • Replaces fixed positioning with flexbox layout in nav-wrapper, primary-nav-items, and secondary-nav-items.
    • Adds flex-direction: column and adjusts flex properties for proper spacing and growth.
    • Removes position: fixed, bottom: 0, and left: 0 from secondary-nav-items.
    • Removes hardcoded max-height constraints causing the gap.
  • Testing:
    • Manually tested with various numbers of navigation items to ensure gap no longer appears and all items remain accessible.

This description was created by Ellipsis for c42c18e. It will automatically update as commits are pushed.

@gabber235 gabber235 requested a review from YounixM as a code owner March 25, 2025 07:59
Copy link

welcome bot commented Mar 25, 2025

Welcome to the SigNoz community! Thank you for your first pull request and making this project better. 🤗

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to c42c18e in 58 seconds

More details
  • Looked at 41 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 5 drafted comments based on config settings.
1. frontend/src/container/SideNav/SideNav.styles.scss:105
  • Draft comment:
    Good use of flexbox on .nav-wrapper and .primary-nav-items to distribute space evenly and enable scrolling. This should resolve the gap issue efficiently.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
2. frontend/src/container/SideNav/SideNav.styles.scss:124
  • Draft comment:
    Removing fixed positioning from .secondary-nav-items in favor of flex properties is a neat fix. Ensure that this change doesn’t impact behavior on different viewport sizes.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
3. frontend/src/container/SideNav/SideNav.styles.scss:107
  • Draft comment:
    Good use of flex properties in .nav-wrapper to distribute space. Verify that the calc(100% - 52px) height works consistently across all viewports.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
4. frontend/src/container/SideNav/SideNav.styles.scss:112
  • Draft comment:
    The additions of display: flex, flex-direction: column, flex: 1, and min-height: 0 in .primary-nav-items ensure a proper scrollable area within the flex container. Looks solid.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
5. frontend/src/container/SideNav/SideNav.styles.scss:125
  • Draft comment:
    The removal of fixed positioning in .secondary-nav-items in favor of flex layout is appropriate for eliminating the gap. Ensure that this change maintains the intended bottom alignment in all scenarios.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None

Workflow ID: wflow_9q7MOXakhI2CLtPr


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@CLAassistant
Copy link

CLAassistant commented Mar 25, 2025

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@aks07 aks07 left a comment

Choose a reason for hiding this comment

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

LGTM. @YounixM

@vikrantgupta25 vikrantgupta25 added the safe-to-test Run CI tests for dependabot and external contributors label Apr 28, 2025
@primus-bot primus-bot bot removed the safe-to-test Run CI tests for dependabot and external contributors label Apr 28, 2025
@vikrantgupta25 vikrantgupta25 added the safe-to-test Run CI tests for dependabot and external contributors label Apr 28, 2025
@vikrantgupta25 vikrantgupta25 enabled auto-merge (squash) April 28, 2025 20:26
@vikrantgupta25 vikrantgupta25 merged commit ef4e3a3 into SigNoz:main Apr 28, 2025
44 of 46 checks passed
Copy link

welcome bot commented Apr 28, 2025

Congrats on merging your first pull request!
minion-party
We here at SigNoz are proud of you! 🥳

shivanshuraj1333 pushed a commit that referenced this pull request May 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safe-to-test Run CI tests for dependabot and external contributors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants