Skip to content

[WEB-4981] fix: analytics portal modal#7858

Merged
pushya22 merged 1 commit intopreviewfrom
fix-analytics-portal-modal
Sep 26, 2025
Merged

[WEB-4981] fix: analytics portal modal#7858
pushya22 merged 1 commit intopreviewfrom
fix-analytics-portal-modal

Conversation

@anmolsinghbhatia
Copy link
Collaborator

@anmolsinghbhatia anmolsinghbhatia commented Sep 26, 2025

Description

This PR includes fixes for analytics portal modal

Type of Change

  • Bug fix

Summary by CodeRabbit

  • Style
    • Updated Work Items modal to be truly edge‑to‑edge in full-screen by removing outer margins.
    • Adjusted modal portal content anchoring to align within its container rather than the viewport, improving positional consistency across layouts.
    • No changes to controls, interactions, or accessibility roles; overall behavior remains the same aside from visual spacing and anchoring improvements.

@anmolsinghbhatia anmolsinghbhatia self-assigned this Sep 26, 2025
Copilot AI review requested due to automatic review settings September 26, 2025 08:44
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 26, 2025

Walkthrough

Adjusts layout classes for the analytics WorkItems modal container and the shared modal portal wrapper: removes conditional margin in full-screen mode and switches portal content positioning from fixed to absolute. No API, state, or event logic changes.

Changes

Cohort / File(s) Summary
Analytics WorkItems Modal
apps/web/core/components/analytics/work-items/modal/index.tsx
Removes adding m-2 when fullScreen is true on the modal root container.
Propel Modal Portal
packages/propel/src/portal/modal-portal.tsx
Changes modal content wrapper positioning from fixed inset-0 to absolute inset-0; other rendering and roles unchanged.

Sequence Diagram(s)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested labels

🐛bug, 🌐frontend, ready_to_merge

Suggested reviewers

  • vamsikrishnamathala
  • sriramveeraghanta

Poem

I hop through portals, light as air,
Full-screen fields with margins rare.
From fixed to absolute I glide,
Modals snug where views reside.
Thump-thump—UX set just right,
Carrot-approved, we ship tonight! 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The description provides a high-level summary and correctly labels the change type but omits the required Screenshots and Media, Test Scenarios, and References sections from the repository template. Without these sections reviewers lack insight into how the fix was validated and any related context. As a result, the description is incomplete against the project’s guidelines. Please add the Screenshots and Media section if applicable, outline the Test Scenarios performed to verify the fix, and include a References section linking any related issues or tickets. This will ensure the PR description fully conforms to the repository’s template and provides reviewers with all necessary context.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly indicates this change is a bug fix for the analytics portal modal and includes the relevant ticket reference without extraneous detail. It succinctly conveys the primary focus of the changeset and adheres to the project’s naming conventions. This makes it easy for team members to scan the history and understand the purpose of the PR at a glance.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-analytics-portal-modal

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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

Copilot AI left a comment

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 fixes CSS positioning and spacing issues in the analytics portal modal to resolve layout problems.

  • Changed modal positioning from fixed to absolute for proper containment
  • Removed margin spacing from fullscreen modal to prevent layout issues

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
packages/propel/src/portal/modal-portal.tsx Changed modal container positioning from fixed to absolute
apps/web/core/components/analytics/work-items/modal/index.tsx Removed margin from fullscreen modal styling

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@makeplane
Copy link

makeplane bot commented Sep 26, 2025

Linked to Plane Work Item(s)

This comment was auto-generated by Plane

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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b946b3a and 85bb77c.

📒 Files selected for processing (2)
  • apps/web/core/components/analytics/work-items/modal/index.tsx (1 hunks)
  • packages/propel/src/portal/modal-portal.tsx (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Build and lint web apps
  • GitHub Check: Build and lint web apps
🔇 Additional comments (1)
apps/web/core/components/analytics/work-items/modal/index.tsx (1)

44-45: LGTM

Dropping the fullscreen margin aligns the content flush with the viewport, which matches the modal layout update. No further action needed.

const content = (
<div
className={cn("fixed inset-0 h-full w-full overflow-y-auto", className)}
className={cn("absolute inset-0 h-full w-full overflow-y-auto", className)}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Restore fixed positioning on the portal container

Switching the wrapper from fixed to absolute means the overlay and dialog now scroll with the document. On long pages the modal slides off-screen as soon as the user scrolls, defeating the purpose of a modal overlay. We still need it anchored to the viewport. Please keep this element position: fixed.

-      className={cn("absolute inset-0 h-full w-full overflow-y-auto", className)}
+      className={cn("fixed inset-0 h-full w-full overflow-y-auto", className)}
🤖 Prompt for AI Agents
In packages/propel/src/portal/modal-portal.tsx around line 92 the wrapper is
using absolute positioning (className includes "absolute"), which causes the
overlay and dialog to scroll with the document; change the positioning back to
fixed by replacing the "absolute" class with "fixed" so the portal is anchored
to the viewport and the modal overlay/dialog stay visible when the page scrolls.

@pushya22 pushya22 merged commit c05a5eb into preview Sep 26, 2025
6 of 9 checks passed
@pushya22 pushya22 deleted the fix-analytics-portal-modal branch September 26, 2025 08:53
yarikoptic pushed a commit to yarikoptic/plane that referenced this pull request Oct 1, 2025
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.

4 participants