This PR fixes responsive layout issues with benchmark charts on narrow mobile screens. #221#228
Conversation
📝 WalkthroughWalkthroughThe changes address responsive rendering issues on narrow mobile viewports by constraining flex layouts with Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/benchmarks/TrendChart.jsx`:
- Around line 39-49: The ResponsiveContainer inside the TrendChart component can
overflow because the parent is a flex column and the title uses flexShrink: 0;
wrap the ResponsiveContainer in a new div (e.g., a flex child wrapper) with
style {flex: 1, minHeight: 0} so the wrapper consumes the remaining space and
allows ResponsiveContainer (width="100%" height="100%") to size correctly;
locate the JSX around ResponsiveContainer in TrendChart.jsx and insert the
wrapper div directly around the ResponsiveContainer element to fix the layout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: bfb2c24f-03e3-4d3b-b467-b7384a7da1ff
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
frontend/src/components/benchmarks/TrendChart.jsxfrontend/src/pages/research/BenchmarksPage.scss
| <div className="trend-chart" style={{ width: '100%', height, display: 'flex', flexDirection: 'column', minWidth: 0 }}> | ||
| <h4 className="trend-chart__title" style={{ | ||
| fontSize: '0.9375rem', | ||
| fontWeight: 600, | ||
| marginBottom: '1rem' | ||
| marginBottom: '1rem', | ||
| flexShrink: 0 | ||
| }}> | ||
| {title} | ||
| </h4> | ||
| <ResponsiveContainer width="100%" height="100%"> | ||
| <ResponsiveContainer width="100%" height="100%" minWidth={0}> | ||
| <LineChart data={formattedData} margin={{ top: 5, right: 30, left: 20, bottom: 5 }}> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate the file and check its size
find . -name "TrendChart.jsx" -type fRepository: Stanzin7/ExtensionShield
Length of output: 119
🏁 Script executed:
# Once found, read the file to see the actual code
cat -n frontend/src/components/benchmarks/TrendChart.jsxRepository: Stanzin7/ExtensionShield
Length of output: 3639
🏁 Script executed:
# Check for CSS files that might style .trend-chart
find ./frontend -name "*.css" -o -name "*.scss" | xargs grep -l "trend-chart" 2>/dev/null | head -5Repository: Stanzin7/ExtensionShield
Length of output: 117
🏁 Script executed:
# Also check if TrendChart is used with height constraints
grep -r "TrendChart" ./frontend --include="*.jsx" --include="*.js" -B 2 -A 2 | head -30Repository: Stanzin7/ExtensionShield
Length of output: 2243
🏁 Script executed:
# Check the SCSS for chart layout constraints
cat ./frontend/src/pages/research/BenchmarksPage.scssRepository: Stanzin7/ExtensionShield
Length of output: 20830
🏁 Script executed:
# Check BenchmarksPage.jsx for actual TrendChart usage with height parameter
cat ./frontend/src/pages/research/BenchmarksPage.jsx | head -150Repository: Stanzin7/ExtensionShield
Length of output: 5675
Wrap ResponsiveContainer in a flex child to prevent chart overflow under fixed-height container.
When TrendChart uses its default height={350}, the flex column allocates the full 350px to ResponsiveContainer via height="100%" while the title (with flexShrink: 0) also consumes space. This causes the chart to overflow its container. Add a wrapper with flex: 1 and minHeight: 0 around ResponsiveContainer to distribute remaining height after the title.
Suggested fix
<h4 className="trend-chart__title" style={{
fontSize: '0.9375rem',
fontWeight: 600,
marginBottom: '1rem',
flexShrink: 0
}}>
{title}
</h4>
+ <div style={{ flex: 1, minHeight: 0, minWidth: 0 }}>
<ResponsiveContainer width="100%" height="100%" minWidth={0}>
<LineChart data={formattedData} margin={{ top: 5, right: 30, left: 20, bottom: 5 }}>
<CartesianGrid strokeDasharray="3 3" stroke="var(--theme-border-subtle)" />
<XAxis
dataKey="date"
stroke="var(--theme-text-subtle)"
style={{ fontSize: '0.75rem' }}
interval="preserveEnd"
minTickGap={20}
angle={-35}
textAnchor="end"
dx={-5}
dy={10}
height={60}
tickLine={false}
/>
<YAxis
stroke="var(--theme-text-subtle)"
style={{ fontSize: '0.75rem' }}
/>
<Tooltip content={<CustomTooltip />} />
<Line
type="monotone"
dataKey="value"
stroke={color}
strokeWidth={2}
dot={{ fill: color, r: 4 }}
activeDot={{ r: 6 }}
isAnimationActive={true}
animationDuration={1200}
animationEasing="ease-out"
/>
</LineChart>
</ResponsiveContainer>
+ </div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className="trend-chart" style={{ width: '100%', height, display: 'flex', flexDirection: 'column', minWidth: 0 }}> | |
| <h4 className="trend-chart__title" style={{ | |
| fontSize: '0.9375rem', | |
| fontWeight: 600, | |
| marginBottom: '1rem' | |
| marginBottom: '1rem', | |
| flexShrink: 0 | |
| }}> | |
| {title} | |
| </h4> | |
| <ResponsiveContainer width="100%" height="100%"> | |
| <ResponsiveContainer width="100%" height="100%" minWidth={0}> | |
| <LineChart data={formattedData} margin={{ top: 5, right: 30, left: 20, bottom: 5 }}> | |
| <div className="trend-chart" style={{ width: '100%', height, display: 'flex', flexDirection: 'column', minWidth: 0 }}> | |
| <h4 className="trend-chart__title" style={{ | |
| fontSize: '0.9375rem', | |
| fontWeight: 600, | |
| marginBottom: '1rem', | |
| flexShrink: 0 | |
| }}> | |
| {title} | |
| </h4> | |
| <div style={{ flex: 1, minHeight: 0, minWidth: 0 }}> | |
| <ResponsiveContainer width="100%" height="100%" minWidth={0}> | |
| <LineChart data={formattedData} margin={{ top: 5, right: 30, left: 20, bottom: 5 }}> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/benchmarks/TrendChart.jsx` around lines 39 - 49, The
ResponsiveContainer inside the TrendChart component can overflow because the
parent is a flex column and the title uses flexShrink: 0; wrap the
ResponsiveContainer in a new div (e.g., a flex child wrapper) with style {flex:
1, minHeight: 0} so the wrapper consumes the remaining space and allows
ResponsiveContainer (width="100%" height="100%") to size correctly; locate the
JSX around ResponsiveContainer in TrendChart.jsx and insert the wrapper div
directly around the ResponsiveContainer element to fix the layout.
|
Hey @tejaswiverma121-byte, it works really well. I really appreciate your fix, however, I feel that this fix might create some issue around 350px. Please go through the code rabbit once and try fixing it. Also, there is one more concern about changes in package-lock.json any particular reason for the change? |
github-actions
Bot
removed
the
area: infra
|
@sapnilbiswas |
sapnilbiswas
left a comment
sapnilbiswas
left a comment
There was a problem hiding this comment.
Thanks, @tejaswiverma121-byte for working on this. Looks good to me
Will be merging soon
2 participants
Changes Made
Issue Fixed
On very narrow viewports (~300px width), benchmark charts were overflowing their cards and overlapping adjacent content, making the layout cluttered and hard to read.
Testing
Tested on:
300px × 611pxVerified that:
Demo
I am attaching video reference
WhatsApp.Video.2026-04-23.at.17.22.31.mp4
Closes #221
Summary by CodeRabbit