-
Notifications
You must be signed in to change notification settings - Fork 204
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enhance/plugin header section redesign #2461
base: develop
Are you sure you want to change the base?
Conversation
WalkthroughThe changes include modifications to the styling of the admin header in Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (2)
templates/admin-header.php (1)
45-56
: Escape output for safetyWhile the
href
attribute is correctly escaped usingesc_url
, it's good practice to also escape theUpgrade
text inside the anchor tag, even if it's plain text, to prevent any potential issues.Apply this minor suggestion:
<a target="_blank" href="<?php echo esc_url( $upgrade_url ); ?>" class="upgrade-button"> - Upgrade + <?php echo esc_html( 'Upgrade' ); ?> <?php dokan_get_template_part( 'svg-icons/pro-award', null, [ 'width' => 20, 'height' => 20, ] ); ?> </a>assets/src/less/admin.less (1)
7-10
: Consistent use of units in CSS propertiesIn the declaration
box-shadow: 0px 1px 5px rgba(0, 0, 0, 0.06);
, the first value0px
can be simplified to0
since zero values do not require units.Apply this minor improvement:
-box-shadow: 0px 1px 5px rgba(0, 0, 0, 0.06); +box-shadow: 0 1px 5px rgba(0, 0, 0, 0.06);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
assets/src/less/admin.less
(6 hunks)templates/admin-header.php
(2 hunks)templates/svg-icons/pro-award.php
(1 hunks)
🔇 Additional comments (1)
assets/src/less/admin.less (1)
271-293
: Check for mobile responsiveness
The responsive styles added within the media query ensure the admin header adapts to smaller screens. Please verify that these changes render correctly on various mobile devices and screen sizes.
Please test the responsive layout on devices with widths up to 782px to ensure the header elements display as intended.
<div class="dokan-admin-logo-wrap"> | ||
<!-- Logo Section --> | ||
<div class="dokan-admin-header-logo"> | ||
<img src="<?php echo esc_url( DOKAN_PLUGIN_ASSEST . '/images/dokan-logo.png' ); ?>" alt="Dokan Logo"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in constant name 'DOKAN_PLUGIN_ASSEST'
It appears that the constant DOKAN_PLUGIN_ASSEST
may be misspelled. It should likely be DOKAN_PLUGIN_ASSETS
to correctly reference the assets directory.
Please apply the following fix:
-<img src="<?php echo esc_url( DOKAN_PLUGIN_ASSEST . '/images/dokan-logo.png' ); ?>" alt="Dokan Logo"/>
+<img src="<?php echo esc_url( DOKAN_PLUGIN_ASSETS . '/images/dokan-logo.png' ); ?>" alt="Dokan Logo"/>
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<img src="<?php echo esc_url( DOKAN_PLUGIN_ASSEST . '/images/dokan-logo.png' ); ?>" alt="Dokan Logo"/> | |
<img src="<?php echo esc_url( DOKAN_PLUGIN_ASSETS . '/images/dokan-logo.png' ); ?>" alt="Dokan Logo"/> |
templates/admin-header.php
Outdated
|
||
<?php | ||
// Show Pro version if installed | ||
if ( dokan()->is_pro_exists() && dokan_pro()->license->is_valid() ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential method call on null object
In the condition if ( dokan()->is_pro_exists() && dokan_pro()->license->is_valid() )
, there is a possibility that dokan_pro()
may return null
if the Pro plugin is not installed. This could lead to a fatal error when attempting to access the license
property.
Consider updating the condition to ensure that dokan_pro()
returns a valid object before accessing its properties:
-if ( dokan()->is_pro_exists() && dokan_pro()->license->is_valid() ) {
+if ( dokan()->is_pro_exists() && dokan_pro() && dokan_pro()->license->is_valid() ) {
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
if ( dokan()->is_pro_exists() && dokan_pro()->license->is_valid() ) { | |
if ( dokan()->is_pro_exists() && dokan_pro() && dokan_pro()->license->is_valid() ) { |
$width = $args['width'] ?? 20; | ||
$height = $args['height'] ?? 20; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initialize $args
to prevent undefined variable notice
To prevent a potential undefined variable notice when $args
is not provided, it's recommended to initialize $args
before using it.
Apply this fix:
<?php
+$args = isset( $args ) ? $args : [];
$width = $args['width'] ?? 20;
$height = $args['height'] ?? 20;
?>
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
$width = $args['width'] ?? 20; | |
$height = $args['height'] ?? 20; | |
<?php | |
$args = isset( $args ) ? $args : []; | |
$width = $args['width'] ?? 20; | |
$height = $args['height'] ?? 20; | |
?> |
templates/svg-icons/pro-award.php
Outdated
<svg width="<?php echo $width; ?>" height="<?php echo $height; ?>" viewBox="0 0 16 16" fill="none" xmlns="http://www.w3.org/2000/svg"> | ||
<path fill-rule="evenodd" clip-rule="evenodd" d="M7.17566 3.52563C7.30278 3.45067 7.44772 3.41113 7.59529 3.41113C7.74291 3.41113 7.8878 3.45067 8.01497 3.52563C8.14197 3.60052 8.24662 3.70803 8.31809 3.837M8.3183 3.8374L9.83426 6.57021C9.83488 6.57112 9.83578 6.57182 9.83686 6.57215C9.83802 6.57253 9.8393 6.57248 9.84041 6.57203C9.84037 6.57207 9.84045 6.57203 9.84041 6.57203L12.4523 5.41061L12.4537 5.41C12.5947 5.34793 12.7502 5.3265 12.9027 5.34814C13.0552 5.36976 13.1986 5.43359 13.3168 5.53241C13.435 5.63123 13.5231 5.76111 13.5714 5.9074C13.6195 6.05328 13.626 6.20966 13.5902 6.35905C13.59 6.35942 13.59 6.35984 13.5899 6.36025L12.2728 11.9539C12.2465 12.0618 12.1989 12.1634 12.1327 12.2527C12.0665 12.3419 11.9831 12.417 11.8874 12.4734C11.7917 12.5299 11.6857 12.5666 11.5756 12.5814C11.4654 12.5962 11.3535 12.5887 11.2463 12.5595L11.2447 12.5591C8.85502 11.8974 6.33044 11.8974 3.94075 12.5591L3.93913 12.5595C3.83194 12.5887 3.71997 12.5962 3.60986 12.5814C3.49974 12.5666 3.3937 12.5299 3.29802 12.4734C3.20233 12.417 3.11892 12.3419 3.05273 12.2527C2.98654 12.1634 2.9389 12.0618 2.91264 11.9539L2.91183 11.9505L1.60073 6.36025C1.60061 6.35971 1.60048 6.35918 1.60036 6.35868C1.56461 6.20941 1.57115 6.05316 1.61923 5.9074C1.66748 5.76111 1.75567 5.63123 1.87382 5.53241C1.99197 5.43359 2.13539 5.36976 2.2879 5.34814C2.4404 5.32651 2.59591 5.34793 2.73688 5.41L2.73827 5.41061L5.35005 6.57199C5.35001 6.57195 5.35009 6.57199 5.35005 6.57199C5.35116 6.57244 5.35257 6.57253 5.35377 6.57215C5.3548 6.57182 5.35571 6.57112 5.35633 6.57021L6.87253 3.837C6.94397 3.70803 7.04862 3.60052 7.17566 3.52563M6.07438 6.97923C5.97249 7.15783 5.80802 7.29235 5.61277 7.3568C5.41747 7.42125 5.20528 7.41109 5.01707 7.32821L5.0157 7.32759L2.40393 6.16624L2.40457 6.16888L3.71559 11.7589C3.71577 11.7594 3.71603 11.7599 3.71638 11.7604C3.71664 11.7607 3.71696 11.7611 3.71732 11.7614C3.7175 11.7615 3.71769 11.7616 3.7179 11.7618C3.7185 11.7621 3.71916 11.7623 3.71983 11.7624C3.72047 11.7625 3.72111 11.7625 3.72173 11.7623C6.25525 11.0611 8.9317 11.0612 11.4652 11.7628L11.3549 12.1609L11.4635 11.7623C11.4642 11.7625 11.4649 11.7625 11.4656 11.7624C11.4663 11.7623 11.4669 11.7621 11.4675 11.7618C11.4681 11.7614 11.4687 11.7609 11.4691 11.7604C11.4693 11.7601 11.4695 11.7597 11.4697 11.7594C11.4697 11.7592 11.4698 11.759 11.4699 11.7588C11.4698 11.7589 11.4699 11.7587 11.4699 11.7588L12.7861 6.16855L12.7867 6.16624L10.1749 7.32759L10.1735 7.32821C9.98534 7.41109 9.77311 7.42125 9.57786 7.3568C9.38256 7.29235 9.21813 7.15783 9.11625 6.97918L9.11381 6.97493L7.59529 4.23743L6.07438 6.97923Z" fill="white"/> | ||
</svg> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure proper escaping of attributes
When outputting variables within HTML attributes, it's important to escape them to prevent security vulnerabilities. In the <svg>
tag, the $width
and $height
attributes should be escaped.
Apply this fix:
-<svg width="<?php echo $width; ?>" height="<?php echo $height; ?>" viewBox="0 0 16 16" fill="none" xmlns="http://www.w3.org/2000/svg">
+<svg width="<?php echo esc_attr( $width ); ?>" height="<?php echo esc_attr( $height ); ?>" viewBox="0 0 16 16" fill="none" xmlns="http://www.w3.org/2000/svg">
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<svg width="<?php echo $width; ?>" height="<?php echo $height; ?>" viewBox="0 0 16 16" fill="none" xmlns="http://www.w3.org/2000/svg"> | |
<path fill-rule="evenodd" clip-rule="evenodd" d="M7.17566 3.52563C7.30278 3.45067 7.44772 3.41113 7.59529 3.41113C7.74291 3.41113 7.8878 3.45067 8.01497 3.52563C8.14197 3.60052 8.24662 3.70803 8.31809 3.837M8.3183 3.8374L9.83426 6.57021C9.83488 6.57112 9.83578 6.57182 9.83686 6.57215C9.83802 6.57253 9.8393 6.57248 9.84041 6.57203C9.84037 6.57207 9.84045 6.57203 9.84041 6.57203L12.4523 5.41061L12.4537 5.41C12.5947 5.34793 12.7502 5.3265 12.9027 5.34814C13.0552 5.36976 13.1986 5.43359 13.3168 5.53241C13.435 5.63123 13.5231 5.76111 13.5714 5.9074C13.6195 6.05328 13.626 6.20966 13.5902 6.35905C13.59 6.35942 13.59 6.35984 13.5899 6.36025L12.2728 11.9539C12.2465 12.0618 12.1989 12.1634 12.1327 12.2527C12.0665 12.3419 11.9831 12.417 11.8874 12.4734C11.7917 12.5299 11.6857 12.5666 11.5756 12.5814C11.4654 12.5962 11.3535 12.5887 11.2463 12.5595L11.2447 12.5591C8.85502 11.8974 6.33044 11.8974 3.94075 12.5591L3.93913 12.5595C3.83194 12.5887 3.71997 12.5962 3.60986 12.5814C3.49974 12.5666 3.3937 12.5299 3.29802 12.4734C3.20233 12.417 3.11892 12.3419 3.05273 12.2527C2.98654 12.1634 2.9389 12.0618 2.91264 11.9539L2.91183 11.9505L1.60073 6.36025C1.60061 6.35971 1.60048 6.35918 1.60036 6.35868C1.56461 6.20941 1.57115 6.05316 1.61923 5.9074C1.66748 5.76111 1.75567 5.63123 1.87382 5.53241C1.99197 5.43359 2.13539 5.36976 2.2879 5.34814C2.4404 5.32651 2.59591 5.34793 2.73688 5.41L2.73827 5.41061L5.35005 6.57199C5.35001 6.57195 5.35009 6.57199 5.35005 6.57199C5.35116 6.57244 5.35257 6.57253 5.35377 6.57215C5.3548 6.57182 5.35571 6.57112 5.35633 6.57021L6.87253 3.837C6.94397 3.70803 7.04862 3.60052 7.17566 3.52563M6.07438 6.97923C5.97249 7.15783 5.80802 7.29235 5.61277 7.3568C5.41747 7.42125 5.20528 7.41109 5.01707 7.32821L5.0157 7.32759L2.40393 6.16624L2.40457 6.16888L3.71559 11.7589C3.71577 11.7594 3.71603 11.7599 3.71638 11.7604C3.71664 11.7607 3.71696 11.7611 3.71732 11.7614C3.7175 11.7615 3.71769 11.7616 3.7179 11.7618C3.7185 11.7621 3.71916 11.7623 3.71983 11.7624C3.72047 11.7625 3.72111 11.7625 3.72173 11.7623C6.25525 11.0611 8.9317 11.0612 11.4652 11.7628L11.3549 12.1609L11.4635 11.7623C11.4642 11.7625 11.4649 11.7625 11.4656 11.7624C11.4663 11.7623 11.4669 11.7621 11.4675 11.7618C11.4681 11.7614 11.4687 11.7609 11.4691 11.7604C11.4693 11.7601 11.4695 11.7597 11.4697 11.7594C11.4697 11.7592 11.4698 11.759 11.4699 11.7588C11.4698 11.7589 11.4699 11.7587 11.4699 11.7588L12.7861 6.16855L12.7867 6.16624L10.1749 7.32759L10.1735 7.32821C9.98534 7.41109 9.77311 7.42125 9.57786 7.3568C9.38256 7.29235 9.21813 7.15783 9.11625 6.97918L9.11381 6.97493L7.59529 4.23743L6.07438 6.97923Z" fill="white"/> | |
</svg> | |
<svg width="<?php echo esc_attr( $width ); ?>" height="<?php echo esc_attr( $height ); ?>" viewBox="0 0 16 16" fill="none" xmlns="http://www.w3.org/2000/svg"> | |
<path fill-rule="evenodd" clip-rule="evenodd" d="M7.17566 3.52563C7.30278 3.45067 7.44772 3.41113 7.59529 3.41113C7.74291 3.41113 7.8878 3.45067 8.01497 3.52563C8.14197 3.60052 8.24662 3.70803 8.31809 3.837M8.3183 3.8374L9.83426 6.57021C9.83488 6.57112 9.83578 6.57182 9.83686 6.57215C9.83802 6.57253 9.8393 6.57248 9.84041 6.57203C9.84037 6.57207 9.84045 6.57203 9.84041 6.57203L12.4523 5.41061L12.4537 5.41C12.5947 5.34793 12.7502 5.3265 12.9027 5.34814C13.0552 5.36976 13.1986 5.43359 13.3168 5.53241C13.435 5.63123 13.5231 5.76111 13.5714 5.9074C13.6195 6.05328 13.626 6.20966 13.5902 6.35905C13.59 6.35942 13.59 6.35984 13.5899 6.36025L12.2728 11.9539C12.2465 12.0618 12.1989 12.1634 12.1327 12.2527C12.0665 12.3419 11.9831 12.417 11.8874 12.4734C11.7917 12.5299 11.6857 12.5666 11.5756 12.5814C11.4654 12.5962 11.3535 12.5887 11.2463 12.5595L11.2447 12.5591C8.85502 11.8974 6.33044 11.8974 3.94075 12.5591L3.93913 12.5595C3.83194 12.5887 3.71997 12.5962 3.60986 12.5814C3.49974 12.5666 3.3937 12.5299 3.29802 12.4734C3.20233 12.417 3.11892 12.3419 3.05273 12.2527C2.98654 12.1634 2.9389 12.0618 2.91264 11.9539L2.91183 11.9505L1.60073 6.36025C1.60061 6.35971 1.60048 6.35918 1.60036 6.35868C1.56461 6.20941 1.57115 6.05316 1.61923 5.9074C1.66748 5.76111 1.75567 5.63123 1.87382 5.53241C1.99197 5.43359 2.13539 5.36976 2.2879 5.34814C2.4404 5.32651 2.59591 5.34793 2.73688 5.41L2.73827 5.41061L5.35005 6.57199C5.35001 6.57195 5.35009 6.57199 5.35005 6.57199C5.35116 6.57244 5.35257 6.57253 5.35377 6.57215C5.3548 6.57182 5.35571 6.57112 5.35633 6.57021L6.87253 3.837C6.94397 3.70803 7.04862 3.60052 7.17566 3.52563M6.07438 6.97923C5.97249 7.15783 5.80802 7.29235 5.61277 7.3568C5.41747 7.42125 5.20528 7.41109 5.01707 7.32821L5.0157 7.32759L2.40393 6.16624L2.40457 6.16888L3.71559 11.7589C3.71577 11.7594 3.71603 11.7599 3.71638 11.7604C3.71664 11.7607 3.71696 11.7611 3.71732 11.7614C3.7175 11.7615 3.71769 11.7616 3.7179 11.7618C3.7185 11.7621 3.71916 11.7623 3.71983 11.7624C3.72047 11.7625 3.72111 11.7625 3.72173 11.7623C6.25525 11.0611 8.9317 11.0612 11.4652 11.7628L11.3549 12.1609L11.4635 11.7623C11.4642 11.7625 11.4649 11.7625 11.4656 11.7624C11.4663 11.7623 11.4669 11.7621 11.4675 11.7618C11.4681 11.7614 11.4687 11.7609 11.4691 11.7604C11.4693 11.7601 11.4695 11.7597 11.4697 11.7594C11.4697 11.7592 11.4698 11.759 11.4699 11.7588C11.4698 11.7589 11.4699 11.7587 11.4699 11.7588L12.7861 6.16855L12.7867 6.16624L10.1749 7.32759L10.1735 7.32821C9.98534 7.41109 9.77311 7.42125 9.57786 7.3568C9.38256 7.29235 9.21813 7.15783 9.11625 6.97918L9.11381 6.97493L7.59529 4.23743L6.07438 6.97923Z" fill="white"/> | |
</svg> |
There was a problem hiding this 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
🧹 Outside diff range and nitpick comments (1)
templates/admin-header.php (1)
27-37
: Consider extracting SVG icons to separate filesThe SVG icons are embedded directly in the HTML, which makes the template harder to maintain. Consider extracting them to separate files and including them using
dokan_get_template_part()
.Also applies to: 45-55
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
templates/admin-header.php
(2 hunks)templates/svg-icons/pro-award.php
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- templates/svg-icons/pro-award.php
🔇 Additional comments (3)
templates/admin-header.php (3)
7-7
: Typo in constant name 'DOKAN_PLUGIN_ASSEST'
The constant name contains a typo: 'ASSEST' should be 'ASSETS'.
Line range hint 1-167
: Overall structure looks good!
The template is well-structured with:
- Proper HTML organization
- Consistent class naming
- Good use of escaping functions
- Clear separation of concerns
22-22
: 🛠️ Refactor suggestion
Improve null check in conditional statement
The current condition could be more defensive. While dokan()->is_pro_exists()
is checked, accessing dokan_pro()->license
could throw an error if dokan_pro()
returns null.
Apply this fix:
-if ( dokan()->is_pro_exists() && dokan_pro() && dokan_pro()->license->is_valid() ) {
+if ( dokan()->is_pro_exists() && dokan_pro() && dokan_pro()->license && dokan_pro()->license->is_valid() ) {
Likely invalid or redundant comment.
$upgrade_url = 'https://dokan.co/wordpress/pricing/'; | ||
?> | ||
<a target="_blank" href="<?php echo esc_url( $upgrade_url ); ?>" class="upgrade-button"> | ||
Upgrade |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Improve external link handling
The upgrade URL should be:
- Configurable rather than hardcoded
- Include proper security attributes for external links
Apply these changes:
-$upgrade_url = 'https://dokan.co/wordpress/pricing/';
+$upgrade_url = apply_filters( 'dokan_admin_upgrade_url', 'https://dokan.co/wordpress/pricing/' );
-<a target="_blank" href="<?php echo esc_url( $upgrade_url ); ?>" class="upgrade-button">
+<a target="_blank" rel="noopener noreferrer" href="<?php echo esc_url( $upgrade_url ); ?>" class="upgrade-button">
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
$upgrade_url = 'https://dokan.co/wordpress/pricing/'; | |
?> | |
<a target="_blank" href="<?php echo esc_url( $upgrade_url ); ?>" class="upgrade-button"> | |
Upgrade | |
$upgrade_url = apply_filters( 'dokan_admin_upgrade_url', 'https://dokan.co/wordpress/pricing/' ); | |
?> | |
<a target="_blank" rel="noopener noreferrer" href="<?php echo esc_url( $upgrade_url ); ?>" class="upgrade-button"> | |
Upgrade |
All Submissions:
Changes proposed in this Pull Request:
Related Pull Request(s)
Closes
How to test the changes in this Pull Request:
Changelog entry
Title
Detailed Description of the pull request. What was previous behaviour
and what will be changed in this PR.
Before Changes
Describe the issue before changes with screenshots(s).
After Changes
Describe the issue after changes with screenshot(s).
Feature Video (optional)
Link of detailed video if this PR is for a feature.
PR Self Review Checklist:
FOR PR REVIEWER ONLY:
Summary by CodeRabbit
New Features
Style