-
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: Update dokan admin dashboard logo & add logo update notice. #2258
Enhance: Update dokan admin dashboard logo & add logo update notice. #2258
Conversation
@shohag121 Vhaiya, kindly review this PR at your convenience. |
This comment was marked as resolved.
This comment was marked as resolved.
…ription & remove one-step notice.
@mrabbani Vhai, need to relevant ui's for new issues. |
This comment was marked as resolved.
This comment was marked as resolved.
Here is the Design Link. Please have a look. |
@tuhin480 Vhai, we need to make consistent between existing banner & updated logo. Here is the preview after updating logo. I think It will be better if we added some shadow or spaces on left/bottom side in this banner image. (as like as previous) We need to update wp org's banner & icon logo's as per latest update. |
This issue was split into two different GitHub links, which caused a delay. I've completed all the tasks and would like to request you to review the design. |
@imtiaz-pranto via, Pls review design and let us know your feedback. |
@shashwatahalder01 vaia, Task:
Resource: Timeline
|
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: 0
🧹 Outside diff range and nitpick comments (3)
includes/Admin/Notices/Manager.php (3)
131-133
: Update the@since
tag with the actual version number.Replace
DOKAN_SINCE
with the actual version number where this feature was introduced.
158-163
: Update the@since
tag with the actual version number.Replace
DOKAN_SINCE
with the actual version number where this feature was introduced.
168-177
: Improve permission check documentation.Consider adding more context to the
@since
tag and parameter documentation to explain whymanage_woocommerce
capability is required.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
includes/Admin/Notices/Manager.php
(3 hunks)
🔇 Additional comments (3)
includes/Admin/Notices/Manager.php (3)
51-52
: LGTM: Hook registration follows WordPress standards.
The new hooks are properly registered and follow WordPress naming conventions.
Line range hint 139-156
: LGTM: Notice implementation follows WordPress standards.
The notice implementation is well-structured with:
- Proper type hints
- Approved content from the content team
- Correct dismissal configuration
177-191
: LGTM: Secure implementation of notice dismissal.
The implementation follows WordPress security best practices:
- Proper nonce verification
- Permission checks
- Data sanitization
Enhance: Update dokan admin logo & add logo update notice.
All Submissions:
Changes proposed in this Pull Request:
Related Pull Request(s)
https://github.com/getdokan/dokan-pro/pull/3738
Closes
How to test the changes in this Pull Request:
Changelog entry
Detailed Description of the pull request. What was previous behaviour
and what will be changed in this PR.
Before Changes
After Changes
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
Release Notes
New Features
Styling Improvements
Bug Fixes
These updates aim to create a more cohesive and user-friendly interface for the admin panel.