Skip to content

Fix modal overflow and improve responsiveness on on small screens of contribute page#3392

Merged
kasya merged 3 commits intoOWASP:mainfrom
Isha-upadhyay:model-description-overflow
Jan 17, 2026
Merged

Fix modal overflow and improve responsiveness on on small screens of contribute page#3392
kasya merged 3 commits intoOWASP:mainfrom
Isha-upadhyay:model-description-overflow

Conversation

@Isha-upadhyay
Copy link
Contributor

Proposed change

Resolves #3369

This PR fixes an issue on the OWASP Nest Contributor page where the “Read more” dialog overflowed the viewport on mobile and small screen devices, making content difficult to read and interact with.

What was changed

  • Made the modal layout fully responsive across all screen sizes
  • Ensured long titles and descriptions wrap correctly instead of overflowing
  • Added proper internal scrolling for modal content on smaller viewports
  • Adjusted modal height constraints to prevent viewport overflow
  • Improved overall usability without changing any backend logic or data flow

Why this change

On smaller screens, the dialog content exceeded the viewport height, causing text and actions to be cut off.
This update ensures the modal remains readable and accessible on mobile devices while preserving the existing desktop experience.


Screenshots

Screenshot 2026-01-17 202809

Testing

  • Verified modal behavior locally using the frontend development server
  • Tested with long titles and content on both desktop and mobile viewports
  • Confirmed content scrolls correctly and no elements overflow outside the modal

Checklist

  • Required: I followed the contributing workflow
  • Required: I verified that my code works as intended and resolves the issue
  • Required: I ran relevant local checks for this UI-only change
  • I used AI for code, documentation, tests, or communication related to this PR

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 17, 2026

Summary by CodeRabbit

  • Style

    • Refined modal styling and classes for consistent spacing and improved dark-mode visuals.
  • Refactor

    • Reorganized into distinct header, scrollable body, and sticky footer; introduced dedicated title/description, summary, and hint sections with updated markdown rendering.
  • New Features

    • Modal size adjusted for improved layout and a visible Close button with accessible labeling and updated button visuals.

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

Refactors frontend/src/components/Modal.tsx: reduces modal size (from 4xl → lg), restructures header/body/footer into responsive flex layout with scrollable body and sticky footer, adds explicit close button, updates Markdown rendering areas (Summary and Hint), and standardizes Tailwind classes including dark-mode variants.

Changes

Cohort / File(s) Summary
Modal component
frontend/src/components/Modal.tsx
Rewrote modal layout and styling: changed size to lg; replaced prior class set with responsive flex layout, max height/width constraints, dark borders/backgrounds, and updated backdrop; refactored header to h2#modal-title with optional description; made body flex-1 with overflow-y-auto, updated Summary and Hint Markdown rendering and styling; removed internal divider; added sticky bottom footer with ActionButton and new ghost Close button (aria-label and explicit label); general Tailwind/dark-mode cleanup.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • arkid15r
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: fixing modal overflow and improving responsiveness on small screens of the contribute page, which directly aligns with the primary objective of the changeset.
Description check ✅ Passed The description is well-detailed and directly related to the changeset, explaining the bug fix, changes made, testing approach, and linking to the resolved issue #3369.
Linked Issues check ✅ Passed The PR successfully addresses all requirements from issue #3369: the modal is made responsive, content wraps properly, internal scrolling is added, and viewport overflow is prevented through comprehensive styling updates to Modal.tsx.
Out of Scope Changes check ✅ Passed All changes in Modal.tsx are directly scoped to fixing the overflow and responsiveness issues described in issue #3369; no unrelated modifications or out-of-scope changes are present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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.

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: 3

🤖 Fix all issues with AI agents
In `@frontend/src/components/Modal.tsx`:
- Around line 73-84: The h2 with id "modal-title" (and the modal body elements
around the other two occurrences) uses a nonstandard Tailwind class
`overflow-wrap-anywhere`; remove that invalid class or replace it with a valid
Tailwind arbitrary utility `overflow-wrap-[anywhere]` (or keep the existing
`break-words` if sufficient), or alternatively define a custom utility in
tailwind.config.js named e.g. `overflow-wrap-anywhere` and use that; update the
className strings in the Modal component (the h2 and the two body sections)
accordingly.
- Around line 133-151: There is a duplicate separator between the modal body and
footer: a manual divider div (the element with className "mx-5 h-px bg-gray-300
dark:bg-gray-700") and a top border applied via the ModalFooter className
(contains "border-t border-gray-200 dark:border-gray-700"); fix by removing one
of them—either delete the manual divider div to keep the footer border, or
remove "border-t border-gray-200 dark:border-gray-700" from the ModalFooter
className so the single manual divider remains.
- Around line 37-58: The Modal contains invalid/duplicate Tailwind classes:
replace the nonstandard backdrop-blur-xs in the Modal's classNames.base with a
valid utility such as backdrop-blur-sm/backdrop-blur/backdrop-blur-md (or remove
it) and replace every usage of overflow-wrap-anywhere (found in ModalContent and
other places) with the correct Tailwind utilities break-words or break-all as
appropriate; also remove the duplicated max-h constraints (max-h-[100vh] and
sm:max-h-[90vh]) from either classNames.base or the ModalContent className so
the max-height appears in only one location.
🧹 Nitpick comments (1)
frontend/src/components/Modal.tsx (1)

158-181: Button variant="ghost" contradicts the solid background styling.

The variant="ghost" typically renders a transparent/text-only button, but the className applies solid background colors (bg-gray-600, dark:bg-gray-700). While the styles will apply, this is semantically inconsistent. Consider using variant="solid" to match the visual intent:

          <Button
-           variant="ghost"
+           variant="solid"
            onPress={onClose}

@sonarqubecloud
Copy link

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.

@Isha-upadhyay this looks great, thank you! 👍🏼

Comment on lines 153 to 169
Copy link
Collaborator

@kasya kasya Jan 17, 2026

Choose a reason for hiding this comment

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

Please run make check-test next time - we don't wrap classes like this and our linters would have picked this up and updated the formatting.
I'll push a change to update this 👌🏼

Copy link
Contributor Author

@Isha-upadhyay Isha-upadhyay Jan 17, 2026

Choose a reason for hiding this comment

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

I did run make check-test locally and the relevant frontend checks passed.

The failures I saw were limited to YAML / line-ending related hooks (CRLF vs LF) which are environment-specific and unrelated to the modal UI change, so I didn’t include them in this PR.

@kasya kasya added this pull request to the merge queue Jan 17, 2026
Merged via the queue into OWASP:main with commit f5a7707 Jan 17, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Contributor “Read more” dialog overflows on mobile and small screens

2 participants

Comments