Skip to content

fix: prevent long non-breaking URLs from overflowing project view (#7…#7961

Merged
qnixsynapse merged 3 commits intojanhq:mainfrom
web-dev0521:fix/long-url-overflow-sidebar
Apr 15, 2026
Merged

fix: prevent long non-breaking URLs from overflowing project view (#7…#7961
qnixsynapse merged 3 commits intojanhq:mainfrom
web-dev0521:fix/long-url-overflow-sidebar

Conversation

@web-dev0521
Copy link
Copy Markdown
Contributor

@web-dev0521 web-dev0521 commented Apr 13, 2026

Describe Your Changes

  • Add min-w-0 to SidebarInset to prevent flex item from expanding beyond viewport when content contains long non-breaking text
  • Add max-w-full overflow-hidden to thread card Link in project view to constrain width
  • Add block truncate to thread title span to truncate long titles with ellipsis

Fixes Issues

Self Checklist

  • Added relevant comments, esp in complex areas
  • Updated docs (for bug fixes / features)
  • Created issues for follow-up changes or refactoring needed

Screenshots

After fix

![thread-title-truncated]
screenshot
(screenshot.png)

Thread titles with long non-breaking text are now properly truncated with ellipsis instead of breaking the container layout.

@web-dev0521
Copy link
Copy Markdown
Contributor Author

Could you please review my PR?

@tokamak-pm
Copy link
Copy Markdown

tokamak-pm bot commented Apr 14, 2026

PR Review: fix: prevent long non-breaking URLs from overflowing project view

Summary of Changes

This PR fixes a UI overflow bug (closes #7959) where long non-breaking text (e.g., URLs without spaces or hyphens) in thread titles would cause the project view to expand horizontally beyond the viewport. Two files are modified with a total of 4 additions and 4 deletions.

File 1: web-app/src/components/ui/sidebar.tsx

  • Adds min-w-0 to SidebarInset's root <main> element. This is a foundational flexbox fix: flex-1 creates a flex item that can grow, but without min-w-0 the default min-width: auto allows the element to grow wider than its flex container when children contain non-breakable content.

File 2: web-app/src/containers/ThreadList.tsx

  • Adds max-w-full overflow-hidden to the project-view thread card <Link> element.
  • Adds block truncate to the thread title <span> to truncate the title text with an ellipsis when it overflows.

Code Quality Assessment

Correctness of the CSS approach

The fix is technically sound and follows well-established Tailwind/CSS patterns:

  1. min-w-0 on SidebarInset — This is the correct, idiomatic fix for flex children that contain overflow content. Without it, the flex-1 flex item obeys min-width: auto and will widen instead of wrapping/truncating. This is a well-known flexbox gotcha.

  2. max-w-full overflow-hidden on the <Link> — Constrains the card to its parent's width and hides overflow. This works in conjunction with min-w-0 higher up in the tree.

  3. block truncate on the <span>truncate is a Tailwind utility that applies overflow: hidden; text-overflow: ellipsis; white-space: nowrap. The block display is required because truncate with white-space: nowrap only takes effect on block-level elements. This is correct.

Minor Observations

  • The whitespace normalisation in ThreadList.tsx changed {currentProjectId ? to {currentProjectId ? (trailing space removed). This is a trivial cosmetic change and does not affect behaviour.
  • The sidebar.tsx file is a shared UI primitive (shadcn-style). Adding min-w-0 is a low-risk, generally-desirable change for a sidebar inset because the overflow problem can appear in many sidebar consumers, not just the thread list. However, maintainers should be aware this is a change to a shared primitive — verify no other usages of SidebarInset depend on the current expansion behaviour.
  • The fix only applies to the project-view branch of the conditional (currentProjectId ?). The non-project branch uses SidebarMenuButton / <Link> without the same overflow guards. Titles in that branch would still overflow if they contain non-breaking text. Consider whether the same block truncate guard should be applied to the <span> inside SidebarMenuButton on line 151 as well for consistency.

Risks and Concerns

  1. Shared primitive changeSidebarInset is a base UI component (likely auto-generated from shadcn/ui). Adding min-w-0 is broadly safe, but if the codebase ever regenerates this file from an upstream shadcn template the change would be overwritten. A comment explaining the reason would help future maintainers.

  2. Truncation UX — Thread titles are now truncated with an ellipsis. Users lose access to the full title text in the card. Check whether a tooltip (title attribute or a <Tooltip> component) should be added to reveal the full text on hover. This is a UX improvement but not a blocker.

  3. overflow-hidden on the card — Adding overflow-hidden to the <Link> card could clip absolutely-positioned child elements. Looking at the surrounding code, SidebarMenuAction (the MoreHorizontal dropdown trigger) is a sibling of <Link> and sits outside it, so this should not cause clipping issues — but a quick visual check is recommended.

  4. max-w-full redundancy — With min-w-0 on the parent SidebarInset and overflow-hidden on the <Link>, max-w-full may be redundant. It is harmless and makes the intent explicit, which is fine.


Missing Tests

This is a purely visual/CSS bug fix with no automated test coverage expected. Recommended manual test matrix:

  • Thread title containing a very long URL with no whitespace (the original repro case).
  • Thread title that is a normal short string (regression: ensure it still renders correctly).
  • Sidebar in both collapsed and expanded states.
  • Mobile viewport (the sidebar variant conditionally changes layout).
  • Non-project-view thread list (verify the SidebarInset change does not break it).

Final Recommendation

Improve needed (minor) before merge:

  1. Consistency gap — The same block truncate overflow protection is not applied to the non-project-view <span> on line 151 in ThreadList.tsx. Thread titles in that context would still overflow for the same class of inputs. Recommend applying the same fix there for a complete resolution.
  2. Accessibility / UX — Consider adding title={thread.title || t('common:newThread')} to the truncated <span> so users can read the full title on hover, since it is now clipped.
  3. Documentation — A short inline comment near the min-w-0 addition in sidebar.tsx explaining it prevents flex overflow would help future maintainers who may regenerate this file from shadcn/ui templates.

The core fix is correct, targeted, and low-risk. The changes are easy to understand and follow standard Tailwind/CSS flexbox patterns. None of the above suggestions are blocking, but items 1 and 2 in particular would make the fix more complete. Good work identifying the root cause and tackling it at the right level of the component tree!

@web-dev0521
Copy link
Copy Markdown
Contributor Author

Hello, @qnixsynapse !
I attached additional Test Screenshot.
Please review my PR.
Thank you.
screenshot-1

@tokamak-pm
Copy link
Copy Markdown

tokamak-pm bot commented Apr 15, 2026

Code Review — follow-up review

New commits since last review: 74cd6af3 (fix: apply truncation and title tooltip to non-project thread titles), 3106c3ab (merge main).

Previous feedback addressed

In the initial review I noted that the overflow fix only applied to the project-view branch of the conditional and that the non-project SidebarMenuButton path still lacked the same block truncate guard. Commit 74cd6af3 directly addresses this:

  • The <span> inside the non-project SidebarMenuButton / <Link> now has className="block truncate" and a title attribute for the tooltip, matching the project-view path.
  • The same title tooltip was also added to the project-view <span>, which is a nice consistency improvement.

Both changes are correct. SidebarMenuButton already constrains width, so block truncate on the inner <span> is sufficient.

Remaining observations

  • The sidebar.tsx change to SidebarInset (adding min-w-0) plus a CSS comment explaining why is unchanged and still looks good.
  • The merge commit 3106c3ab is a clean merge from main with no conflict-resolution artifacts.
  • No new issues introduced.

Merge readiness

All feedback from the initial review has been addressed. The fix is minimal, correct, and consistently applied to both the project-view and non-project thread title paths.

Recommendation: can merge

@web-dev0521
Copy link
Copy Markdown
Contributor Author

Thank you for your review.

@qnixsynapse
Copy link
Copy Markdown
Contributor

This one looks good. Thank you for contributing. Will merge after testing later today. Make sure to add necessary tests.

qnixsynapse
qnixsynapse previously approved these changes Apr 15, 2026
@qnixsynapse qnixsynapse force-pushed the fix/long-url-overflow-sidebar branch from 806a03b to 9fbc6c8 Compare April 15, 2026 05:52
@web-dev0521
Copy link
Copy Markdown
Contributor Author

Hi, @qnixsynapse !
Thank you for your attention.
I've just added test code.
Please take a few minutes to review my test code.
Thank you.

@qnixsynapse qnixsynapse merged commit e601157 into janhq:main Apr 15, 2026
18 checks passed
@github-project-automation github-project-automation bot moved this to QA in Jan Apr 15, 2026
@web-dev0521
Copy link
Copy Markdown
Contributor Author

Hello, @qnixsynapse!
Thank you for taking the time.
I will contribute to this repository as much as possible.

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

Projects

Status: QA

Development

Successfully merging this pull request may close these issues.

bug: Long non-breaking links break container inside the project

2 participants