Skip to content

Conversation

@ktx-vaidehi
Copy link
Collaborator

@ktx-vaidehi ktx-vaidehi commented Aug 29, 2025

PR Type

Bug fix


Description

  • Fix sidebar height overflow in panel sidebar

  • Remove redundant inline height on right column

  • Ensure viewport-based sizing for config panel


Diagram Walkthrough

flowchart LR
  View["AddPanel.vue layout"] -- "drop inline height" --> Column["Right column .col-auto"]
  Column -- "hosts" --> Sidebar["PanelSidebar.vue .sidebar-content"]
  Sidebar -- "set viewport height" --> CSS["height: calc(100vh - 176px)"]
Loading

File Walkthrough

Relevant files
Bug fix
PanelSidebar.vue
Use viewport-based height for sidebar content                       

web/src/components/dashboards/addPanel/PanelSidebar.vue

  • Replace flex/min-height with fixed viewport-based height
  • Keep vertical scroll via overflow-y: auto
+1/-2     
AddPanel.vue
Clean inline height to prevent overflow issues                     

web/src/views/Dashboards/addPanel/AddPanel.vue

  • Remove inline style height: 100% on right column
  • Simplify layout to rely on CSS sizing
+2/-2     

@github-actions github-actions bot added ☢️ Bug Something isn't working Review effort 2/5 labels Aug 29, 2025
@github-actions
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Responsive Height

Using a hardcoded viewport subtraction (100vh - 176px) might not account for different header/toolbars or browser UI changes, potentially causing cutoff or extra space. Validate across breakpoints, mobile, and when horizontal scrollbars appear.

.sidebar-content {
  padding: 0px 10px;
  height: calc(100vh - 176px);
  overflow-y: auto;
Layout Regression

Removing the inline height from the right column relies on the sidebar’s internal height and overflow. Ensure the parent flex container and separators don’t cause the sidebar to collapse or overflow when the config panel toggles or on small screens.

<div class="col-auto">
  <PanelSidebar
    :title="t('dashboard.configLabel')"
    v-model="dashboardPanelData.layout.isConfigPanelOpen"
  >

@github-actions
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Use flexible height instead of fixed

Avoid hardcoding viewport height with a magic pixel offset; it can break with
varying headers, toolbars, or zoom and cause double scrollbars on smaller screens.
Use flex growth with min-height: 0 so the container adapts within its parent and
still scrolls correctly. This keeps layout resilient across breakpoints.

web/src/components/dashboards/addPanel/PanelSidebar.vue [138-142]

 .sidebar-content {
   padding: 0px 10px;
-  height: calc(100vh - 176px);
+  flex: 1 1 auto;
+  min-height: 0;
   overflow-y: auto;
 }
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly targets the new height: calc(100vh - 176px); and proposes reverting to a flex-based approach with min-height: 0, which is generally more robust and avoids viewport magic numbers; impact is moderate to high but context-dependent on parent flex layout.

Medium
Replace fixed vh height with flex

Tying the editor container to a viewport-based height with a fixed offset risks
overflow conflicts and layout issues on small screens. Prefer flex layout sizing so
it shares space with siblings and scrolls as needed within the page.

web/src/views/Dashboards/addPanel/AddPanel.vue [275-276]

-<div class="row column tw-h-[calc(100vh-180px)]">
+<div class="row column" style="min-height: 0; flex: 1 1 auto;">
   <DashboardQueryEditor />
 </div>
Suggestion importance[1-10]: 6

__

Why: It accurately identifies the new tw-h-[calc(100vh-180px)] usage and suggests a flex-based alternative to prevent overflow issues; beneficial but may require ensuring parent containers use flex for full effect.

Low
Prevent sidebar height collapse

Removing style="height: 100%;" without adjusting parent sizing can cause the sidebar
to collapse or not fill available space. Ensure the column grows and can scroll by
using flex basis and min-height rules aligned with the content container.

web/src/views/Dashboards/addPanel/AddPanel.vue [279-284]

-<div class="col-auto">
+<div class="col-auto" style="flex: 0 0 auto; min-height: 0;">
   <PanelSidebar
     :title="t('dashboard.configLabel')"
     v-model="dashboardPanelData.layout.isConfigPanelOpen"
   >
Suggestion importance[1-10]: 5

__

Why: The concern about removing height: 100% is valid, and adding min-height: 0 can help, but flex: 0 0 auto on col-auto may limit desired resizing; improvement is plausible yet somewhat speculative without full layout context.

Low

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Summary

This PR fixes height-related layout issues in the dashboard configuration panel by modifying the CSS layout approach in two Vue components. The changes target the OpenObserve dashboard's "Add Panel" interface, specifically addressing how the configuration sidebar handles vertical spacing.

The primary change removes inline style="height: 100%;" from two div containers in AddPanel.vue that wrap PanelSidebar components. These containers are used for both query-related charts (line 279) and custom charts (line 460). By removing this constraint, the containers can now size themselves naturally using the existing col-auto Bootstrap class and flex layout system.

The complementary change in PanelSidebar.vue replaces the flexible CSS layout approach with a fixed height calculation. The .sidebar-content class previously used flex: 1 and min-height: 0 to allow flexible growth within the flex container. This has been replaced with height: calc(100vh - 176px) which provides a fixed height calculation. The 176px offset accounts for various UI elements above the content area, including the header (which has a 60px height based on .sidebar-header-expanded), separators, padding, and other chrome elements.

These changes work together to provide better control over the dashboard configuration panel's layout. The removal of forced 100% height on containers prevents overflow issues, while the fixed height calculation ensures the sidebar content area has predictable dimensions for proper scrolling and content display. This approach shifts from a purely flexible layout to a more controlled one, addressing the height calculation problems that were likely causing the dashboard configuration interface to render incorrectly.

Confidence score: 4/5

  • This PR addresses a specific layout issue with targeted CSS changes that should resolve height problems without breaking existing functionality
  • Score reflects straightforward styling changes with clear intent, though the magic number 176px creates some brittleness
  • Pay close attention to the fixed height calculation in PanelSidebar.vue as it may need adjustment if surrounding UI elements change

2 files reviewed, no comments

Edit Code Review Bot Settings | Greptile

@ktx-vaidehi ktx-vaidehi merged commit ef1bf59 into main Aug 29, 2025
40 of 54 checks passed
@ktx-vaidehi ktx-vaidehi deleted the fix/dashboard/config-height-issue branch August 29, 2025 10:57
ktx-vaidehi added a commit that referenced this pull request Aug 29, 2025
### **PR Type**
Bug fix


___

### **Description**
- Fix sidebar height overflow in panel sidebar

- Remove redundant inline height on right column

- Ensure viewport-based sizing for config panel


___

### Diagram Walkthrough


```mermaid
flowchart LR
  View["AddPanel.vue layout"] -- "drop inline height" --> Column["Right column .col-auto"]
  Column -- "hosts" --> Sidebar["PanelSidebar.vue .sidebar-content"]
  Sidebar -- "set viewport height" --> CSS["height: calc(100vh - 176px)"]
```



<details> <summary><h3> File Walkthrough</h3></summary>

<table><thead><tr><th></th><th align="left">Relevant
files</th></tr></thead><tbody><tr><td><strong>Bug
fix</strong></td><td><table>
<tr>
  <td>
    <details>
<summary><strong>PanelSidebar.vue</strong><dd><code>Use viewport-based
height for sidebar content</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary>
<hr>

web/src/components/dashboards/addPanel/PanelSidebar.vue

<ul><li>Replace flex/min-height with fixed viewport-based height<br>
<li> Keep vertical scroll via overflow-y: auto</ul>


</details>


  </td>
<td><a
href="https://github.com/openobserve/openobserve/pull/8201/files#diff-8e63f9f02875ae5308688ef83d5a8d12383dd32b298475e85a210d6c7334af79">+1/-2</a>&nbsp;
&nbsp; &nbsp; </td>

</tr>

<tr>
  <td>
    <details>
<summary><strong>AddPanel.vue</strong><dd><code>Clean inline height to
prevent overflow issues</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary>
<hr>

web/src/views/Dashboards/addPanel/AddPanel.vue

<ul><li>Remove inline style height: 100% on right column<br> <li>
Simplify layout to rely on CSS sizing</ul>


</details>


  </td>
<td><a
href="https://github.com/openobserve/openobserve/pull/8201/files#diff-c9632db1ad95af95aba10161bcbc7c5608318919f228637bdafae3f3a31267e8">+2/-2</a>&nbsp;
&nbsp; &nbsp; </td>

</tr>
</table></td></tr></tr></tbody></table>

</details>

___
ktx-vaidehi added a commit that referenced this pull request Aug 29, 2025
### **PR Type**
Bug fix


___

### **Description**
- Fix sidebar height overflow in panel sidebar

- Remove redundant inline height on right column

- Ensure viewport-based sizing for config panel


___

### Diagram Walkthrough


```mermaid
flowchart LR
  View["AddPanel.vue layout"] -- "drop inline height" --> Column["Right column .col-auto"]
  Column -- "hosts" --> Sidebar["PanelSidebar.vue .sidebar-content"]
  Sidebar -- "set viewport height" --> CSS["height: calc(100vh - 176px)"]
```



<details> <summary><h3> File Walkthrough</h3></summary>

<table><thead><tr><th></th><th align="left">Relevant
files</th></tr></thead><tbody><tr><td><strong>Bug
fix</strong></td><td><table>
<tr>
  <td>
    <details>
<summary><strong>PanelSidebar.vue</strong><dd><code>Use viewport-based
height for sidebar content</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary> <hr>

web/src/components/dashboards/addPanel/PanelSidebar.vue

<ul><li>Replace flex/min-height with fixed viewport-based height<br>
<li> Keep vertical scroll via overflow-y: auto</ul>


</details>


  </td>
<td><a

href="https://github.com/openobserve/openobserve/pull/8201/files#diff-8e63f9f02875ae5308688ef83d5a8d12383dd32b298475e85a210d6c7334af79">+1/-2</a>&nbsp;
&nbsp; &nbsp; </td>

</tr>

<tr>
  <td>
    <details>
<summary><strong>AddPanel.vue</strong><dd><code>Clean inline height to
prevent overflow issues</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary> <hr>

web/src/views/Dashboards/addPanel/AddPanel.vue

<ul><li>Remove inline style height: 100% on right column<br> <li>
Simplify layout to rely on CSS sizing</ul>


</details>


  </td>
<td><a

href="https://github.com/openobserve/openobserve/pull/8201/files#diff-c9632db1ad95af95aba10161bcbc7c5608318919f228637bdafae3f3a31267e8">+2/-2</a>&nbsp;
&nbsp; &nbsp; </td>

</tr>
</table></td></tr></tr></tbody></table>

</details>

___
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

☢️ Bug Something isn't working Review effort 2/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants