-
Notifications
You must be signed in to change notification settings - Fork 711
fix: dashboard config height issue #8201
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
Conversation
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
There was a problem hiding this 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
### **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>
</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>
</td>
</tr>
<tr>
<td>
<details>
<summary><strong>AddPanel.vue</strong><dd><code>Clean inline height to
prevent overflow issues</code>
</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>
</td>
</tr>
</table></td></tr></tr></tbody></table>
</details>
___
### **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>
</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>
</td>
</tr>
<tr>
<td>
<details>
<summary><strong>AddPanel.vue</strong><dd><code>Clean inline height to
prevent overflow issues</code>
</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>
</td>
</tr>
</table></td></tr></tr></tbody></table>
</details>
___
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
File Walkthrough
PanelSidebar.vue
Use viewport-based height for sidebar contentweb/src/components/dashboards/addPanel/PanelSidebar.vue
AddPanel.vue
Clean inline height to prevent overflow issuesweb/src/views/Dashboards/addPanel/AddPanel.vue