Fix color picker PRO badge icon and hover overlay in settings #1771
Conversation
WalkthroughThis PR updates the color-picker rendering to optionally wrap inputs for pro-preview, expands allowed wp_kses tags for inline SVG/div, adds CSS/LESS for a non-interactive pro-preview overlay, and replaces crown SVG file reads with an image reference for the pro badge across admin settings and some front-end templates. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
arifulhoque7
added
needs: dev review
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Lib/WeDevs_Settings_API.php (1)
533-547: Consider removing redundant inline styles.The wrapper logic is correct, but the inline styles on Line 538 are redundant since
.wpuf-color-picker-wrapperalready has these styles defined in the CSS (lines 895-900 in admin.less).You can simplify Line 538:
- $html .= '<div class="wpuf-color-picker-wrapper" style="position: relative; display: inline-block;">'; + $html .= '<div class="wpuf-color-picker-wrapper">';
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Lib/WeDevs_Settings_API.php(2 hunks)assets/css/admin.css(1 hunks)assets/less/admin.less(1 hunks)includes/functions/settings-options.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
includes/functions/settings-options.php (1)
wpuf-functions.php (1)
wpuf_is_pro_active(5042-5044)
🔇 Additional comments (3)
assets/less/admin.less (1)
894-945: LGTM! Well-structured pro preview styles.The color picker pro preview styling is well-implemented with proper layering, hover states, and interaction controls. The use of
pointer-eventsto disable interaction andz-indexfor overlay positioning follows best practices.assets/css/admin.css (1)
690-733: LGTM! Compiled CSS matches LESS source.The compiled CSS correctly reflects the LESS source changes for the color picker pro preview styles. All selectors, properties, and values are properly compiled.
Lib/WeDevs_Settings_API.php (1)
561-572: LGTM! Proper expansion of allowed HTML.The allowed HTML expansion correctly permits the wrapper div, SVG elements, and path elements needed for the pro preview rendering. All attributes are appropriate and necessary.
Updated the 'pro' badge image in login form settings options to use esc_url for the image source and esc_attr__ for the alt attribute, improving security and proper escaping.
|
Some pro batch got under the reference line @arifulhoque7 vai |
This is expected behavior. Since the label is relatively large, there isn’t enough space for it to remain on the same line. @Rubaiyat-E-Mohammad vai |
|
We can use "BG" instead of Background |
Updated the labels for login form color options to use shorter names: 'Form BG Color', 'Field BG Color', and 'Button BG Color' instead of the longer 'Background Color' variants. This improves label clarity and consistency in the settings UI.
|
done @Rubaiyat-E-Mohammad vai, @anik-fahmid vai |
Rubaiyat-E-Mohammad
added
QA Approved
Updated the registration form to use an image for the PRO crown icon instead of inline SVG. Adjusted CSS to support both SVG and image display for the icon.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
includes/Free/templates/page-registration-form.php (2)
195-195: Fix the incorrect wp_kses usage and align with the img tag approach.This line has multiple critical issues:
Incorrect sanitization order:
wp_kses()is applied to the file path$crown_iconbeforefile_get_contents(). This makes no sense—wp_kses()sanitizes HTML content, not file paths. The current implementation will likely fail to read the file.Security risk: Even if the order were corrected, echoing
file_get_contents()output requires proper sanitization of the SVG content to prevent XSS vulnerabilities.Inconsistency: Line 148 correctly uses an
<img>tag for the same pro badge, but this line attempts to inline the SVG content usingfile_get_contents(). The PR objectives indicate a shift to using<img>tags.🔎 Recommended fix: Use an img tag for consistency and security
Replace the inline SVG approach with an
<img>tag matching line 148:- <span class="pro-icon icon-white"> <?php echo file_get_contents( wp_kses($crown_icon, array('svg' => [ 'xmlns' => true, 'width' => true, 'height' => true, 'viewBox' => true, 'fill' => true ], 'path' => [ 'd' => true, 'fill' => true ], 'circle' => [ 'cx' => true, 'cy' => true, 'r' => true ], ) ) ); // @codingStandardsIgnoreLine ?></span> + <span class="pro-icon icon-white"> + <img src="<?php echo esc_url( WPUF_ASSET_URI . '/images/pro-badge.svg' ); ?>" alt="<?php esc_attr_e( 'PRO', 'wp-user-frontend' ); ?>"> + </span>If you must inline the SVG, fix the sanitization order:
- <span class="pro-icon icon-white"> <?php echo file_get_contents( wp_kses($crown_icon, array('svg' => [ 'xmlns' => true, 'width' => true, 'height' => true, 'viewBox' => true, 'fill' => true ], 'path' => [ 'd' => true, 'fill' => true ], 'circle' => [ 'cx' => true, 'cy' => true, 'r' => true ], ) ) ); // @codingStandardsIgnoreLine ?></span> + <span class="pro-icon icon-white"> <?php echo wp_kses( file_get_contents( $crown_icon ), array('svg' => [ 'xmlns' => true, 'width' => true, 'height' => true, 'viewBox' => true, 'fill' => true ], 'path' => [ 'd' => true, 'fill' => true ], 'circle' => [ 'cx' => true, 'cy' => true, 'r' => true ], ) ); ?></span>However, the first approach (using
<img>tag) is strongly recommended for consistency with the rest of the PR.
100-100: Use esc_url() instead of wp_kses() for sanitizing URLs.The
wp_kses()function is designed to sanitize HTML content, not URLs or file paths. Forsrcattributes containing URLs, useesc_url()for proper sanitization.🔎 Proposed fix
Line 100:
- <img src="<?php echo wp_kses( WPUF_ASSET_URI . '/images/wpuf-pro-2.svg', array('svg' => [ 'xmlns' => true, 'width' => true, 'height' => true, 'viewBox' => true, 'fill' => true ], 'path' => [ 'd' => true, 'fill' => true ],) ); ?>" alt="WPUF Pro"> + <img src="<?php echo esc_url( WPUF_ASSET_URI . '/images/wpuf-pro-2.svg' ); ?>" alt="WPUF Pro">Line 106:
- <img src="<?php echo wp_kses( WPUF_ASSET_URI . '/images/doc.svg', array('svg' => [ 'xmlns' => true, 'width' => true, 'height' => true, 'viewBox' => true, 'fill' => true ], 'path' => ['fill-rule' => true, 'clip-rule' => true, 'd' => true, 'fill' => true ] ) ); ?>" alt=""> + <img src="<?php echo esc_url( WPUF_ASSET_URI . '/images/doc.svg' ); ?>" alt="">Lines 135-142:
- <img src="<?php echo wp_kses( WPUF_ASSET_URI . '/images/form-banner.svg', array( - 'svg' => [ 'xmlns' => true, 'width' => true, 'height' => true, 'viewBox' => true, 'fill' => true ], - 'path' => [ 'd' => true, 'fill' => true, 'fill-opacity' => true, 'fill-rule' => true, 'clip-rule' => true,], - 'rect' => [ 'x' => true, 'y' => true, 'width' => true, 'height' => true, 'rx' => true, 'fill' => true, 'stroke' => true, 'stroke-width' => true], - 'circle' => [ 'cx' => true, 'cy' => true, 'r' => true, 'fill' => true ], - 'linearGradient' => [ 'id' => true, 'x1' => true, 'y1' => true, 'x2' => true, 'y2' => true, 'gradientUnits' => true ], - 'stop' => [ 'stop-color' => true, 'offset' => true, 'stop-opacity' => true] - )); ?>" alt="WPUF Registration Form"> + <img src="<?php echo esc_url( WPUF_ASSET_URI . '/images/form-banner.svg' ); ?>" alt="WPUF Registration Form">Also applies to: 106-106, 135-142
🧹 Nitpick comments (1)
includes/Free/templates/page-registration-form.php (1)
2-2: Consider removing this variable after fixing line 195.The
$crown_iconvariable is only used on line 195 in a problematic implementation. If line 195 is updated to use an<img>tag (like line 148), this variable will become unused and should be removed.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
assets/css/registration-forms.cssincludes/Free/templates/page-registration-form.php
🔇 Additional comments (2)
assets/css/registration-forms.css (1)
110-111: LGTM! Selector correctly supports both SVG and img rendering.The expansion of the selector to include both
svgandimgelements appropriately supports the transition from inline SVG to image-based pro badge rendering.includes/Free/templates/page-registration-form.php (1)
148-148: LGTM! Correct implementation of the pro badge using an img tag.The use of
<img>withesc_url()and a properly escapedaltattribute follows WordPress security best practices and aligns with the PR objectives.
3 participants
Close #1765 ,Related PRO PR
Fix color picker PRO badge icon and hover overlay in settings
Updates:
Files modified:
Technical changes:
Result:
Summary by CodeRabbit
New Features
Style
✏️ Tip: You can customize this high-level summary in your review settings.