improve sticky header button UI/UX #95
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds animated paybutton markup and CSS (overlay hover, logging-out spinner, label swap), updates logout JS to accept the clicked button element and use promise-style success/fail handling that toggles the logging state, and inlines sanitized SVG icons for the buttons. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Button as Logout Button
participant Handler as handleLogout(logoutButton)
participant Server
participant Browser as DOM
User->>Button: Click
Button->>Handler: handleLogout(this)
Handler->>Browser: disable button, add .is-logging-out, show spinner
Handler->>Server: POST /logout
alt Success
Server-->>Handler: 200 OK
Handler->>Browser: set isLoggedIn = false
Handler->>Browser: reload page
else Failure
Server-->>Handler: Error
Handler->>Browser: enable button, remove .is-logging-out
Handler->>User: alert error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
templates/public/sticky-header.php (1)
28-34: Add file existence checks and consider sanitizing SVG output.Same security and error handling concerns apply here as with the Profile button. The logout SVG should also be validated before use.
🧹 Nitpick comments (1)
assets/js/paybutton-paywall-cashtab-login.js (1)
55-55: Consider providing more specific error information.The generic alert message doesn't help users understand what went wrong. If possible, include error details from the server response to aid troubleshooting.
-.fail(function () { +.fail(function (jqXHR, textStatus, errorThrown) { if (logoutButton) { logoutButton.disabled = false; logoutButton.classList.remove('is-logging-out'); } - alert('Logout failed. Please try again.'); + const errorMsg = jqXHR.responseJSON?.data?.message || errorThrown || 'Unknown error'; + alert('Logout failed: ' + errorMsg + '. Please try again.'); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
assets/icons/logout.svgis excluded by!**/*.svgassets/icons/profile.svgis excluded by!**/*.svg
📒 Files selected for processing (3)
assets/css/sticky-header.css(2 hunks)assets/js/paybutton-paywall-cashtab-login.js(1 hunks)templates/public/sticky-header.php(1 hunks)
🔇 Additional comments (3)
templates/public/sticky-header.php (1)
27-39: Good implementation of logout state management.The button structure with separate text spans for default and "logging out" states is well-designed. Passing
thistohandleLogout()enables proper state management.assets/css/sticky-header.css (2)
76-117: Well-implemented logout loading state.The spinner animation and text swapping logic are cleanly implemented. The use of CSS-only state management (hiding/showing text spans) keeps the solution simple and performant.
139-171: Clean animated overlay implementation.The slide-in animation effect is well-structured with proper z-index layering to keep text above the animated overlay. Good use of CSS custom properties for theming.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
paybutton.php(1 hunks)templates/public/sticky-header.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- templates/public/sticky-header.php
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
templates/public/sticky-header.php (2)
12-38: Reuse existingpaybutton_inline_svg()helper instead of redefining SVG allowlist.This block safely sanitizes SVGs, but it reintroduces a local
$paybutton_svg_allowedplus file handling that are already encapsulated bypaybutton_inline_svg( $filename )inpaybutton.php. That duplication risks the two allowlists drifting over time.Consider simplifying to something like:
-<?php - $paybutton_svg_allowed = array( - // ... - ); -?> +<?php + // SVG sanitization and file existence checks are handled centrally + // by paybutton_inline_svg( $filename ) in paybutton.php. +?>and for the icons:
- <span class="btn-icon"> - <?php - $paybutton_svg_path = PAYBUTTON_PLUGIN_DIR . 'assets/icons/profile.svg'; - if ( file_exists( $paybutton_svg_path ) ) { - echo wp_kses( file_get_contents( $paybutton_svg_path ), $paybutton_svg_allowed ); - } - ?> - </span> + <span class="btn-icon"> + <?php echo paybutton_inline_svg( 'profile' ); ?> + </span> - <span class="btn-icon"> - <?php - $paybutton_svg_path = PAYBUTTON_PLUGIN_DIR . 'assets/icons/logout.svg'; - if ( file_exists( $paybutton_svg_path ) ) { - echo wp_kses( file_get_contents( $paybutton_svg_path ), $paybutton_svg_allowed ); - } - ?> - </span> + <span class="btn-icon"> + <?php echo paybutton_inline_svg( 'logout' ); ?> + </span>If the new icons require additional SVG elements/attributes, you can extend the central allowlist in
paybutton_inline_svg()instead of maintaining a second one here.Also applies to: 47-63
66-68: Improve accessibility of the “Logging out…” state.
aria-hidden="true"on the.btn-text-logging-outspan means assistive technologies will never announce that state, even when it becomes visible.Consider either:
- Removing
aria-hiddenand relying on CSSdisplay: none/blockto hide/show the label, or- Using an
aria-live="polite"region for the logging-out text so screen readers announce the status change when logout starts.This keeps the new logging-out UI helpful for keyboard/screen-reader users as well.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
paybutton.php(1 hunks)templates/public/sticky-header.php(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- paybutton.php
|
Bot's feedback applied and ready for review. |
This PR implements #94 and #80 by adding SVG icons for the Profile and Logout buttons, introducing a proper “logging out” visual state with an animated spinner, and adding a smooth slide-hover animation to both buttons.
Test Plan
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.