Skip to content

Fix breadcrumb overflow for long chapter names on small screens#2972

Merged
kasya merged 1 commit intoOWASP:mainfrom
anurag2787:truncate-breadcrumb
Dec 21, 2025
Merged

Fix breadcrumb overflow for long chapter names on small screens#2972
kasya merged 1 commit intoOWASP:mainfrom
anurag2787:truncate-breadcrumb

Conversation

@anurag2787
Copy link
Contributor

@anurag2787 anurag2787 commented Dec 19, 2025

Resolves #2960

Description

This PR fixes an issue where long chapter names cause the breadcrumb to overflow and break the layout on smaller screens.

image

Checklist

  • I read and followed the contributing guidelines
  • I ran make check-test locally and all tests passed
  • I used AI for code, documentation, or tests in this PR

@anurag2787 anurag2787 marked this pull request as ready for review December 19, 2025 12:05
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 19, 2025

Walkthrough

The PR wraps breadcrumb titles with the TruncatedText component to enable text truncation on smaller screens. Changes include adding TruncatedText imports and wrappers in BreadCrumbs and BreadCrumbsWrapper components, adding a min-w-0 class for proper truncation behavior, and updating the test selector to reference the parent element.

Changes

Cohort / File(s) Summary
Breadcrumb component refactoring
frontend/src/components/BreadCrumbs.tsx, frontend/src/components/BreadCrumbsWrapper.tsx
Wrapped breadcrumb titles (both current page and navigation items) with TruncatedText component to apply responsive max-width constraints and truncation behavior. Added TruncatedText import and min-w-0 class to base Breadcrumbs item class for proper text overflow handling.
Test selector adjustment
frontend/__tests__/unit/components/BreadCrumbs.test.tsx
Updated test to select the parent element of the last breadcrumb item instead of the item itself for stylesheet class assertions (cursor-default, font-semibold).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Straightforward component wrapping pattern applied consistently across multiple files
  • Test changes are localized selector adjustments
  • No complex logic or control flow modifications introduced

Possibly related PRs

Suggested labels

frontend, frontend-tests

Suggested reviewers

  • kasya
  • arkid15r

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the main change: fixing breadcrumb overflow for long chapter names on small screens, which aligns with the PR's primary objective.
Linked Issues check ✅ Passed The code changes implement truncation via the TruncatedText component with responsive max-width constraints and enable proper truncation behavior with min-w-0, directly addressing the overflow issue described in issue #2960.
Out of Scope Changes check ✅ Passed All changes are scoped to breadcrumb components and directly address the overflow issue. The test update reflects the implementation changes without introducing unrelated modifications.
Description check ✅ Passed The description is directly related to the changeset, explaining that the PR fixes breadcrumb overflow for long chapter names on small screens, which aligns with the code changes wrapping titles in TruncatedText component.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
frontend/src/components/BreadCrumbs.tsx (1)

20-20: Missing min-w-0 breaks truncation behavior.

The base class is missing min-w-0, which was added in BreadCrumbsWrapper.tsx (line 24). Without this, flexbox items cannot shrink below their content width, preventing the truncation from working properly.

🔎 Proposed fix
           itemClasses={{
-            base: 'transition-colors duration-200',
+            base: 'transition-colors duration-200 min-w-0',
             item: 'text-sm font-medium',
             separator: 'flex items-center',
           }}
🧹 Nitpick comments (1)
frontend/src/components/BreadCrumbs.tsx (1)

5-5: TruncatedText usage is correct, but verify consistency.

The TruncatedText implementation looks good and matches the pattern in BreadCrumbsWrapper.tsx. However, note these minor inconsistencies between the two components:

  • Line 29: Missing isDisabled={isLast} prop (present in BreadCrumbsWrapper line 33)
  • Line 3: Imports FaChevronRight from 'react-icons/fa' while BreadCrumbsWrapper uses 'react-icons/fa6'
  • Line 32: Uses text-gray-600 dark:text-gray-300 while BreadCrumbsWrapper uses text-gray-800 dark:text-gray-100

If these differences are intentional (e.g., different design requirements for the two components), please disregard. Otherwise, consider aligning them for consistency.

Also applies to: 35-48

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c685bed and bc0c717.

📒 Files selected for processing (3)
  • frontend/__tests__/unit/components/BreadCrumbs.test.tsx (1 hunks)
  • frontend/src/components/BreadCrumbs.tsx (2 hunks)
  • frontend/src/components/BreadCrumbsWrapper.tsx (3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-13T07:31:06.511Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: frontend/src/components/ModuleCard.tsx:53-55
Timestamp: 2025-07-13T07:31:06.511Z
Learning: In Next.js 13+ app router, useRouter from 'next/navigation' does not provide asPath or query properties. Use useParams to extract route parameters and usePathname to get the current pathname instead.

Applied to files:

  • frontend/src/components/BreadCrumbsWrapper.tsx
🧬 Code graph analysis (2)
frontend/src/components/BreadCrumbs.tsx (1)
frontend/src/components/TruncatedText.tsx (1)
  • TruncatedText (3-45)
frontend/src/components/BreadCrumbsWrapper.tsx (1)
frontend/src/components/TruncatedText.tsx (1)
  • TruncatedText (3-45)
🔇 Additional comments (3)
frontend/src/components/BreadCrumbsWrapper.tsx (2)

8-8: LGTM! Truncation implementation is correct.

The TruncatedText component is properly imported and applied to both the last item and non-last items with responsive max-width constraints. This will effectively prevent overflow on small screens while allowing full display on larger screens (768px+).

Also applies to: 36-49


24-24: Essential: min-w-0 enables flexbox truncation.

The addition of min-w-0 to the base item class is critical. In flexbox layouts, items have min-width: auto by default, preventing them from shrinking below their content size. Setting min-w-0 allows the items to shrink, which is necessary for the truncation to work properly.

frontend/__tests__/unit/components/BreadCrumbs.test.tsx (1)

56-57: LGTM! Test correctly adapted to new DOM structure.

The test now checks the parent element for styling classes, which is correct since TruncatedText wraps the text in an additional span. The styling classes cursor-default and font-semibold are applied to the parent span (line 32 in BreadCrumbs.tsx), so this assertion properly validates the expected behavior.

Copy link
Collaborator

@kasya kasya left a comment

Choose a reason for hiding this comment

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

@anurag2787 this looks great! Thank you!

@kasya kasya enabled auto-merge December 21, 2025 18:54
@kasya kasya added this pull request to the merge queue Dec 21, 2025
Merged via the queue into OWASP:main with commit 3531fa6 Dec 21, 2025
26 checks passed
Mr-Rahul-Paul pushed a commit to Mr-Rahul-Paul/Nest that referenced this pull request Jan 2, 2026
Co-authored-by: anurag2787 <anurag2787@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Long chapter name breaks breadcrumb layout on smaller screens

2 participants

Comments