Skip to content

Conversation

AbhiVarde
Copy link

@AbhiVarde AbhiVarde commented Oct 11, 2025

What does this PR do?

Fixes z-index stacking context issues where popover menus in settings cards get hidden behind subsequent cards.

Problem

Multiple Card.Base components with default styling create competing stacking contexts. When a popover opens in an upper card (like Git configuration or Global variables), it gets rendered behind lower cards in the DOM, making it partially or completely hidden.

Solution

Added an optional zIndex prop to the CardGrid component. This allows specific cards containing popover menus to establish proper stacking context without affecting all CardGrid instances.

Changes Made

  • Added optional zIndex prop to CardGrid component (src/lib/components/cardGrid.svelte)
  • Applied zIndex={10} to Git configuration card in updateInstallations.svelte
  • Applied zIndex={10} to Global variables card in updateVariables.svelte
  • Maintained backward compatibility - cards without the prop work as before

Test Plan

Local Testing Performed:

  1. ✅ Navigated to Project Settings page
  2. ✅ Added a Git installation and clicked the three-dot menu - verified popover appears on top
  3. ✅ Clicked three-dot menu on Global variables - verified popover appears on top
  4. ✅ Tested other pages using CardGrid component - no visual regression
  5. ✅ Tested in both light and dark themes
  6. ✅ Verified MCP server card (without zIndex) still renders correctly
  7. ✅ Ran pnpm lint and pnpm format - all checks passed

Screenshots

Git Configuration

Before Fix:
git-before

After Fix:
git-after

Global Variables

Before Fix:
global-before

After Fix:
global-after

Related PRs and Issues

Fixes #2464

Additional Notes

The zIndex prop is optional and defaults to undefined, ensuring backward compatibility. Only cards that explicitly need elevated stacking (those with popovers) should use this prop. This is a non-breaking change.

Have you read the Contributing Guidelines on issues?

Yes, I have read and followed the contributing guidelines.

Summary by CodeRabbit

  • New Features
    • CardGrid now supports two new props: style (string) and zIndex (number) for custom inline styling and stacking control.
    • Conditional z-index styling applied when zIndex is provided, ensuring correct layering without altering behavior.
    • Updated Git configuration and project variables views to use zIndex for CardGrid, improving visual stacking in those sections.

- Add optional zIndex prop to CardGrid component
- Apply z-index to Git configuration card for proper popover visibility
- Apply z-index to Global variables card for proper popover visibility
- Ensure popover menus appear above all card elements

Fixes appwrite#2464
Copy link
Contributor

coderabbitai bot commented Oct 11, 2025

Walkthrough

  • Added two public props to src/lib/components/cardGrid.svelte: style (string) and zIndex (number | undefined).
  • Updated rendering to apply position: relative and a z-index when zIndex is defined, alongside the provided style.
  • Updated src/routes/(console)/project-[region]-[project]/settings/updateInstallations.svelte to use in the Git configuration section.
  • Updated src/routes/(console)/project-[region]-[project]/updateVariables.svelte to use in the project variables view.
  • No other public entities were modified.

Suggested reviewers

  • HarshMN2345

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title clearly states that it fixes z-index stacking issues for popovers in settings cards, which aligns precisely with the primary change of adding a zIndex prop and applying it to resolve card stacking conflicts. It is concise, specific, and directly related to the code modifications without extraneous details or file listings. Therefore, it satisfies the criteria for a descriptive and focused title.
Linked Issues Check ✅ Passed The changes implement the core requirement of issue #2464 by extending CardGrid with an optional zIndex prop and applying it to the Git Configuration and Global Variables cards so that popover menus now appear above other cards. Manual testing and linting confirm that the fix works without regressions, and no additional coding objectives were left unaddressed. This directly satisfies the coding goals outlined in the linked issue.
Out of Scope Changes Check ✅ Passed All modifications are confined to adding the zIndex prop to the CardGrid component and updating its usage in the two relevant settings pages; there are no unrelated or extraneous changes outside the scope of resolving popover stacking issues. This PR does not introduce any deviations from the objectives or affect other parts of the codebase.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d3b6653 and c349db3.

📒 Files selected for processing (3)
  • src/lib/components/cardGrid.svelte (1 hunks)
  • src/routes/(console)/project-[region]-[project]/settings/updateInstallations.svelte (1 hunks)
  • src/routes/(console)/project-[region]-[project]/updateVariables.svelte (1 hunks)
🔇 Additional comments (4)
src/lib/components/cardGrid.svelte (2)

6-7: LGTM! Props are well-typed and backward compatible.

The addition of optional style and zIndex props is clean and maintains backward compatibility with existing CardGrid usages.


10-10: Implementation is correct and solves the stacking context issue.

The dynamic inline style correctly applies position: relative and z-index only when needed, which is the right approach for establishing a stacking context. The concatenation with the style prop provides flexibility for additional custom styles.

One edge case to be aware of: if a caller passes style="position: absolute;" or similar positioning, the last declaration would win in CSS, potentially overriding the position: relative and breaking the z-index stacking context. However, current usage doesn't pass conflicting position styles, so this is not a concern in practice.

src/routes/(console)/project-[region]-[project]/updateVariables.svelte (1)

268-268: LGTM! Correct usage of the new zIndex prop.

The zIndex={10} application to the variables CardGrid correctly addresses the reported popover stacking issue. The value is consistent with other usage in the codebase and was verified through manual testing.

src/routes/(console)/project-[region]-[project]/settings/updateInstallations.svelte (1)

80-80: LGTM! Targeted fix for Git configuration popover stacking.

The zIndex={10} application to the Git configuration CardGrid correctly resolves the reported issue where popover menus were hidden behind subsequent cards (e.g., the MCP server card at line 204). The selective application—only to the card that needs it—is the right approach.


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.

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.

🐛 Bug: Popover menus hidden behind cards in settings page

1 participant