Skip to content

Bugfix: Long output content causes full page scroll instead of box sc…#235

Merged
spring1843 merged 6 commits into
spring1843:mainfrom
waseemkhan00777:fix/issue-153-long-content-scroll
Dec 6, 2025
Merged

Bugfix: Long output content causes full page scroll instead of box sc…#235
spring1843 merged 6 commits into
spring1843:mainfrom
waseemkhan00777:fix/issue-153-long-content-scroll

Conversation

@waseemkhan00777

Copy link
Copy Markdown
Contributor

…roll.

Pull Request

📋 Description

Long output content causes full page scroll instead of box scroll.
In many tools, we had the content pull page scroll issue.
The fix is provided to restrict the full page scroll.
Max height is added to common textarea component to handle it.

🔧 Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 📚 Documentation update
  • 🧹 Code refactoring (no functional changes)
  • ⚡ Performance improvement
  • 🧪 Test addition or modification
  • 🔧 Build/CI configuration change

📱 Screenshots/Videos

image

📝 Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my code
  • I have commented my code where necessary
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published

🔗 Related Issues

#153

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR attempts to fix a scrolling issue where long output content causes full page scroll instead of limiting scroll to the textarea box. The fix adds a maxHeight prop to the TextArea component with a default value of "500px".

Key changes:

  • Add maxHeight prop to TextAreaProps interface
  • Set default maxHeight value to "500px"
  • Pass maxHeight to the underlying CodeMirror component

Comment thread client/src/components/ui/textarea.tsx Outdated
theme = "light",
lineWrapping = false,
fixedHeight = true,
maxHeight = "500px",

Copilot AI Nov 30, 2025

Copy link

Choose a reason for hiding this comment

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

The new maxHeight prop with a default value of "500px" conflicts with the existing heightLimitExtension mechanism (lines 283-289).

When fixedHeight=true (the default), the component already sets a dynamic max-height via the heightLimitExtension based on line count:

  • Desktop: ~5400px (300 lines × 18px)
  • Mobile: ~1800px (100 lines × 18px)

The new maxHeight="500px" default will override this existing behavior, effectively breaking the line-based height limiting. This means all textareas will now be capped at 500px regardless of the fixedHeight setting.

Recommendation: Consider one of these approaches:

  1. Only pass maxHeight to CodeMirror when fixedHeight=false, or
  2. Set maxHeight default to undefined and only apply it when explicitly provided by consumers, or
  3. Conditionally apply maxHeight based on fixedHeight: maxHeight={isFixedHeight ? undefined : maxHeight}

This maintains backward compatibility while allowing consumers to opt into a fixed max-height when needed.

Suggested change
maxHeight = "500px",
maxHeight,

Copilot uses AI. Check for mistakes.
Comment thread client/src/components/ui/textarea.tsx Outdated
readOnly?: boolean;
autoFocus?: boolean;
rows?: number;
maxHeight?: string;

Copilot AI Nov 30, 2025

Copy link

Choose a reason for hiding this comment

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

The new maxHeight prop is missing documentation. Following the pattern of the fixedHeight prop (line 108), consider adding a comment to explain its purpose and interaction with other height-related props.

For example:

maxHeight?: string; // Maximum height in CSS units (e.g., "500px"). Note: when fixedHeight is true, this may conflict with line-based height limiting.
Suggested change
maxHeight?: string;
maxHeight?: string; // Maximum height in CSS units (e.g., "500px"). Note: when fixedHeight is true, this may conflict with line-based height limiting.

Copilot uses AI. Check for mistakes.
@netlify

netlify Bot commented Nov 30, 2025

Copy link
Copy Markdown

Deploy Preview for freedevtool ready!

Name Link
🔨 Latest commit 878b0c3
🔍 Latest deploy log https://app.netlify.com/projects/freedevtool/deploys/693393ef041d5f00087c0ca4
😎 Deploy Preview https://deploy-preview-235--freedevtool.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@spring1843

Copy link
Copy Markdown
Owner

@waseemkhan00777 this screenshot appears old, can you please try this problem in the main branch and see if it still exists?

@spring1843 spring1843 merged commit aaae05f into spring1843:main Dec 6, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants