Skip to content

Conversation

HarshMN2345
Copy link
Member

@HarshMN2345 HarshMN2345 commented Oct 16, 2025

What does this PR do?

(Provide a description of what this PR does.)

Test Plan

image

Related PRs and Issues

(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)

Have you read the Contributing Guidelines on issues?

(Write your answer here.)

Summary by CodeRabbit

  • New Features

    • Modal component gains a backdrop toggle to allow modeless (non-blocking) display.
    • Verification email dialog now uses the modeless option where applicable.
  • Chores

    • Updated internal UI component packages to newer builds.

Copy link

railway-app bot commented Oct 16, 2025

This PR was not deployed automatically as @HarshMN2345 does not have access to the Railway project.

In order to get automatic PR deploys, please add @HarshMN2345 to your workspace on Railway.

Copy link
Contributor

coderabbitai bot commented Oct 16, 2025

Walkthrough

This pull request updates two package.json dependencies—@appwrite.io/pink-icons-svelte and @appwrite.io/pink-svelte—from build hash 8f82877 to 10305c4. It adds an exported boolean prop backdrop (default true) to the reusable modal component and forwards it into the inner Modal. The sendVerificationEmailModal component now calls the Modal with backdrop={false} (and retains autoClose={false}). No other logic, error handling, or control flow changes were made.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "feat: Set mode modeless for sendVerificationEmailModal to allow clickthrough overlay for password managers" is directly related to the main changes in the changeset. The primary modifications involve adding a backdrop property to the modal component and setting it to false in sendVerificationEmailModal, which conceptually aligns with making the modal "modeless" by allowing click-through interactions with the overlay for password managers. The title clearly identifies the specific component being modified and explains the purpose of the change. While the changeset also includes dependency updates to pink-icons-svelte and pink-svelte, these appear to be incidental changes that don't detract from the clear primary focus described in the title.
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
  • Commit unit tests in branch fix-SER-458-modal-blocking-password-managers

📜 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 aa9f918 and 2ff2a68.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (1)
  • package.json (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • package.json
⏰ 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
  • GitHub Check: e2e

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: 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 1590206 and e71d9df.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (3)
  • package.json (1 hunks)
  • src/lib/components/account/sendVerificationEmailModal.svelte (1 hunks)
  • src/lib/components/modal.svelte (2 hunks)
🔇 Additional comments (4)
src/lib/components/modal.svelte (1)

33-33: LGTM: Prop correctly forwarded.

The mode prop is properly forwarded to the underlying Modal component using Svelte's shorthand syntax.

src/lib/components/account/sendVerificationEmailModal.svelte (2)

138-139: Verify modeless mode works with custom scrim overlay.

Setting mode="modeless" allows click-through on the modal itself, but the custom .email-verification-scrim overlay (lines 177-188) might still block interactions with password managers beneath it.

Test that password managers can be accessed through the overlay when this modal is displayed. If clicks are still blocked, consider adding pointer-events: none; to .email-verification-scrim and pointer-events: auto; to the modal content.


177-188: Potential conflict: Custom scrim may block click-through.

The custom scrim overlay with backdrop-filter and background-color might prevent the intended click-through behavior for password managers, even with mode="modeless".

Verify that the scrim allows interactions with underlying elements (like password managers) when the modal is modeless. If not, you may need to adjust the CSS:

.email-verification-scrim {
    position: fixed;
    top: 0;
    left: 0;
    width: 100%;
    height: 100%;
    background-color: hsl(240 5% 8% / 0.6);
    backdrop-filter: blur(4px);
    display: flex;
    align-items: center;
    justify-content: center;
+   pointer-events: none; /* Allow click-through for modeless behavior */
}

+/* Restore pointer events on modal content */
+.email-verification-scrim :global(dialog) {
+   pointer-events: auto;
+}
package.json (1)

27-29: Request commit link for Modal mode prop support
I couldn’t find any changelog or release notes for b93a4a4. Please share the GitHub repo/commit URL or changelog snippet where the Modal component’s mode prop was added.

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 e71d9df and 8fece89.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (3)
  • package.json (1 hunks)
  • src/lib/components/account/sendVerificationEmailModal.svelte (1 hunks)
  • src/lib/components/modal.svelte (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/lib/components/account/sendVerificationEmailModal.svelte
⏰ 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). (1)
  • GitHub Check: build
🔇 Additional comments (2)
src/lib/components/modal.svelte (2)

17-17: Well-defined prop with sensible default.

The modeless boolean prop is properly typed and defaults to false, maintaining backward compatibility with existing usages.


33-33: Prop forwarding looks correct.

The modeless prop is correctly forwarded to the inner Modal component using Svelte's shorthand syntax. Ensure this is tested with a password manager to confirm the click-through behavior works as intended for the use case described in the PR objectives.

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 8fece89 and aa9f918.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (3)
  • package.json (1 hunks)
  • src/lib/components/account/sendVerificationEmailModal.svelte (1 hunks)
  • src/lib/components/modal.svelte (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/lib/components/account/sendVerificationEmailModal.svelte
  • package.json

@Meldiron Meldiron merged commit b7ca43d into main Oct 21, 2025
5 checks passed
@Meldiron Meldiron deleted the fix-SER-458-modal-blocking-password-managers branch October 21, 2025 07:43
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