Skip to content

Fix add initiative form being cut off#735

Merged
4tal merged 3 commits into4tals:mainfrom
4tala:fix/add-initiative-form-size
Feb 8, 2026
Merged

Fix add initiative form being cut off#735
4tal merged 3 commits into4tals:mainfrom
4tala:fix/add-initiative-form-size

Conversation

@4tala
Copy link
Copy Markdown
Contributor

@4tala 4tala commented Feb 8, 2026

Problem

The 'Add Initiative' form was being cut in half/not fully visible.

Solution

  • Removed hardcoded iframe dimensions (width=300, height=520) that were overriding CSS
  • Increased form container max-width from 400px to 640px
  • Increased form container height from 600px/80vh to 85vh/800px max
  • Added title attribute to iframe for accessibility

Testing

  • ✅ Lint passes
  • ✅ Build passes

- Remove hardcoded iframe dimensions (300x520), let CSS handle sizing
- Increase form container max-width from 400px to 640px
- Increase form container height from 600px/80vh to 85vh/800px
- Add title attribute to iframe for accessibility
@netlify
Copy link
Copy Markdown

netlify bot commented Feb 8, 2026

Deploy Preview for linksforisrael ready!

Name Link
🔨 Latest commit f1a0f40
🔍 Latest deploy log https://app.netlify.com/projects/linksforisrael/deploys/6988af473b6c960008b0eb99
😎 Deploy Preview https://deploy-preview-735--linksforisrael.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.

@qodo-code-review
Copy link
Copy Markdown

Review Summary by Qodo

Fix Add Initiative form visibility and responsive sizing

🐞 Bug fix ✨ Enhancement

Grey Divider

Walkthroughs

Description
• Removed hardcoded iframe dimensions to allow CSS-based responsive sizing
• Increased form container max-width from 400px to 640px for better visibility
• Adjusted form container height from 600px/80vh to 85vh/800px for optimal display
• Added accessibility title attribute to iframe element
Diagram
flowchart LR
  A["Hardcoded iframe<br/>dimensions 300x520"] -->|Remove| B["CSS-controlled<br/>sizing"]
  C["Form container<br/>400px max-width"] -->|Increase to| D["640px max-width"]
  E["Form container<br/>600px/80vh height"] -->|Adjust to| F["85vh/800px height"]
  G["iframe element"] -->|Add title| H["Accessibility<br/>attribute"]
  B --> I["Responsive<br/>form display"]
  D --> I
  F --> I
Loading

Grey Divider

File Changes

1. app/components/AddSite/AddSite.module.scss ✨ Enhancement +3/-3

Increase form container dimensions for visibility

• Updated .formContainer max-width from 400px to 640px
• Changed height from 600px to 85vh for better vertical space utilization
• Modified max-height from 80vh to 800px for consistent sizing constraints

app/components/AddSite/AddSite.module.scss


2. app/components/AddSite/AddSite.tsx 🐞 Bug fix +1/-5

Remove hardcoded iframe dimensions and add accessibility

• Removed hardcoded width="300" and height="520" attributes from iframe
• Removed frameBorder, marginHeight, and marginWidth attributes
• Added title="Add Initiative Form" attribute for accessibility

app/components/AddSite/AddSite.tsx


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review bot commented Feb 8, 2026

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Remediation recommended

1. 85vh modal height risk 🐞 Bug ⛯ Reliability
Description
• The modal container height was changed to height: 85vh with overflow: hidden, which can lead
  to content being partially clipped on mobile browsers where vh doesn’t match the *visible*
  viewport (dynamic URL/status bars, safe-area insets).
• This is more likely to show up in mobile landscape or when browser UI expands/collapses, because
  the overlay is fixed and centered, leaving little tolerance if vh overestimates available space.
Code

app/components/AddSite/AddSite.module.scss[R103-108]

+	max-width: 640px;
+	height: 85vh;
+	max-height: 800px;
background: white;
overflow: hidden;
box-shadow: 0 25px 50px -12px rgba(0, 0, 0, 0.25);
Evidence
The container is a fixed, centered overlay and the .formContainer is explicitly sized and clips
overflow; if vh overestimates the visible viewport on some mobile browsers, the modal can extend
past the visible area while still hiding overflow.

app/components/AddSite/AddSite.module.scss[84-119]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The modal uses `height: 85vh` and `overflow: hidden`. On some mobile browsers, `vh` can be based on a viewport that includes browser UI, which can cause the modal to extend beyond the visible area while clipping overflow.
### Issue Context
This modal is rendered inside a fixed full-screen overlay and centered via flexbox. The container is a column flex layout and clips overflow.
### Fix Focus Areas
- app/components/AddSite/AddSite.module.scss[100-108]
### Suggested change (example)
- Prefer `dvh` (dynamic viewport height) with a fallback:
- `height: min(85dvh, 800px);`
- keep a fallback `height: 85vh;` before it for older browsers
- Optionally account for safe areas:
- `max-height: calc(100dvh - env(safe-area-inset-top) - env(safe-area-inset-bottom) - 32px);`
- Consider `overflow: auto` on the container (or a wrapper) if you want to guarantee access to content when viewport is constrained.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

4tala added 2 commits February 8, 2026 16:33
The modal was trapped inside a sticky footer with overflow:hidden,
causing it to be clipped. Using createPortal to render modal
directly in document.body fixes the overlay/positioning issue.
@4tal 4tal merged commit 4b0c143 into 4tals:main Feb 8, 2026
5 checks 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.

2 participants